lgtm! it might be good to fire a PR with this change + the engine cache test unskipped and retry that in travis until you repro the prior hang to verify this new exception handling is indeed catching and surfacing the error in the hung context.
[engine] bug fix: ensure we catch all exceptions in subprocess
Review Request #3656 — Created April 6, 2016 and submitted
|kwlzn, patricklaw, stuhood|
We intended to catch all subprocesses exceptions, for example the test case
EngineTest.test_multiprocess_unpickleable checks SerializationError, but some
later added code wasn't protected, including line #159 that caused
https://github.com/pantsbuild/pants/issues/3149 test to hang.
This review fixes this by moving everything into the same try block.
This review does not fix the root cause of https://github.com/pantsbuild/pants/issues/3149
but next time it happens the subprocess won't die and will report the
exception back to the engine.