[engine] bug fix: ensure we catch all exceptions in subprocess

Review Request #3656 — Created April 6, 2016 and submitted

peiyu
pants
3149, 3155
pants-reviews
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.

https://travis-ci.org/peiyuwang/pants/builds/121229553 passed.

  1. 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.

    1. Yesterday when I moved just line 159 to the try block I couldn't repro the pickling error, but let me try again with this review

      Running https://travis-ci.org/peiyuwang/pants/builds/121246651

  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

submitted as f18ce8f4131a85be845ec3becf6d164cfcbdb8bf

Loading...