Fix unit tests hanging when there's an exception in a sandbox run

Fixes #298 (closed)

Rather than having to go through extra effort to reproduce and debug when this happens, fixing the hang lets us see the cause during automated tests.

It's possible that it would be neater to make buildstream._signals.terminator call the cleanup function as well during __exit__. It would reduce the scope of the race condition between removing the signal handler and calling the cleanup manually, but that just moves it to inside buildstream._signals.terminator, so a more complicated solution would be required.

The code looks like a good fix to me.

It would be really nice if we had some sample code which triggers the exceptions to happen in a test case and test that we actually dont hang in these cases...

If that's not possible, I'd be happy to merge this as is.

@tristanvb !329 (merged) has a test that will do that.

added 9 commits

• 1d132807...37d97414 - 8 commits from branch master
• 621d0d85 - mount: Wrap yields in context managers with try-finally

Compare with previous version

enabled an automatic merge when the pipeline for 621d0d85 succeeds

:/ that test didn't fail before the rebase

Peculiarly, running just that test in the same version locally builds.

canceled the automatic merge

It's possible that it would be neater to make buildstream._signals.terminator call the cleanup function as well during __exit__.

Looking into this, I'm wondering, is there a reason we can't simply call sys.exit() in the SIGTERM handler and completely rely on finally? I.e., why do we need the terminator stack in the first place?

terminator() will make a fast exit from a unix signal handler.

finally will never run.

added 10 commits

• 621d0d85...be10c324 - 9 commits from branch master
• 600e17e1 - mount: Wrap yields in context managers with try-finally

Compare with previous version

I left things running a few hours to try and replicate the test failures without success.

I could get the push test to reliably fail locally, but that happened without my changes as well, so I excluded them from tests, and couldn't cause the stack test to fail.

added 4 commits

• 600e17e1...337fc965 - 3 commits from branch master
• e3e72616 - mount: Wrap yields in context managers with try-finally

Compare with previous version

enabled an automatic merge when the pipeline for e3e72616 succeeds

merged