Better handling of exceptions in children, especially for threads. Ad… #27
Conversation
|
Changes Unknown when pulling c954c74 on thread-hang into ** on master**. |
…d unittests for this. Fixes #26.
|
Changes Unknown when pulling 9943a97 on thread-hang into ** on master**. |
| @@ -11,7 +11,8 @@ | |||
| # FOR A PARTICULAR PURPOSE. | |||
| # | |||
| ############################################################################## | |||
| """Multiprocessing utilities. | |||
| """ | |||
There was a problem hiding this comment.
Technically, this is a PEP 8 violation.
Suggest adding a backsplash.
There was a problem hiding this comment.
I prefer the Django convention which doesn't require an opening sentence to be beside the quotes.
| def run_in_child(func, strategy, *args, **kw): | ||
| """Call a function in a child process. Don't return anything. | ||
| def run_in_child(func, strategy, *args, **kwargs): | ||
| """ |
There was a problem hiding this comment.
ditto and ditto for other cases :)
| "process running %r failed with exit code %d" % (func, p.exitcode)) | ||
| Process, Queue = strategies[strategy] | ||
|
|
||
| queue = Queue() |
There was a problem hiding this comment.
Should this Queue be used for the child queue?
There was a problem hiding this comment.
No, that's the parent_queue (for communication to the parent). There is no child queue (communication to the child) because these functions have never been allowed to call sync (which is where the child queue would be used).
There was a problem hiding this comment.
I don't see where the global Queue used by Child.init is defined. I must have missed it.
There was a problem hiding this comment.
The subclass SynclessQueue that we use here passes in a dummy function for that argument.
|
Changes Unknown when pulling 257b832 on thread-hang into ** on master**. |
2 similar comments
|
Changes Unknown when pulling 257b832 on thread-hang into ** on master**. |
|
Changes Unknown when pulling 257b832 on thread-hang into ** on master**. |
|
I just did a quick drive by. I need to understand how this works a lot more to do more of a review and I don't have time for that atm. |
|
Thanks for your comments! In empirical testing, this solves the hanging problems I experienced in #26, and it has tests, so I'm happy enough with it to merge (once I actually turn on coverage for the new tests :/) |
|
Changes Unknown when pulling b06f9a4 on thread-hang into ** on master**. |
…d unittests for this. Fixes #26.