This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/62/

src/rolekit/async.py (Diff revision 1)
193
    return f

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…


- Miloslav Trmac


On srpen 1st, 2014, 9:02 odp. CEST, Stephen Gallagher wrote:

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
By Stephen Gallagher.

Updated Srp. 1, 2014, 9:02 odp.

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.

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.

Diffs

  • src/rolekit/async.py (1e2b82eab5a4e6862594672f738c1935a83c2be5)

View Diff