On Srp. 1, 2014, 11:14 odp., Miloslav Trmac wrote:
> src/rolekit/async.py, line 193
>
<
http://reviewboard-fedoraserver.rhcloud.com/r/62/diff/1/?file=253#file253...
>
> Actually, this might be a major issue with using threads.
>
> The way Future objects work, set_result()/set_exception() directly calls all the
registered callbacks, which include async_step above, and that calls the next step within
the coroutine.
>
> So, when the worker thread finishes working with the subprocess, it calls
future.set_result() and that ends up causing _the next part of the coroutine calling
async_subprocess_ to run within that thread as well, completely unsynchronized with any
other async code in roled and quite possibly accessing the same data (e.g. the instance
objects).
>
> Using the GLib design of
thread-safety-by-no-locking-and-executing-everything-on-the-same-thread, we would need to
ensure that the next part of the coroutine runs on the main thread, i.e. creating a
GSource that poll()s for something set by the thread worker; using (logically) a condition
variable, or (usual implementation?) a helper pipe and unix_fd_source_new.
>
> This is starting to get complicated…
… because I am probably overcomplicating it, it’s too late. The worker thread can call
the Python equivalent of g_idle_add() to cause code to be run on the main process (and the
code run on the main process would be what calls set_result or a set_exception on the
future returned from async_subprocess; and that’s a different future from the one returned
by .submit(), we don’t really need that one AFAICT).
- Miloslav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/62/#review208
-----------------------------------------------------------
On Srp. 1, 2014, 9:02 odp., Stephen Gallagher wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/62/
-----------------------------------------------------------
(Updated Srp. 1, 2014, 9:02 odp.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas
Woerner.
Repository: rolekit
Description
-------
This will spawn a thread to monitor the subprocess, capture both
stdout and stderr and throw an exception if the process terminates
with a non-zero error code.
Diffs
-----
src/rolekit/async.py 1e2b82eab5a4e6862594672f738c1935a83c2be5
Diff:
http://reviewboard-fedoraserver.rhcloud.com/r/62/diff/
Testing
-------
Manual testing was done by having it call a script that slept for 30s and verifying that
other requests could be successfully made to rolekit while it was waiting.
This code probably needs a unit test, but I haven't written it yet.
Thanks,
Stephen Gallagher