Arvados

Branch 11002-arvput-crash-fix at e2ea7599812d12dd027930069ee92e8816ee9de8
Test run: https://ci.curoverse.com/job/developer-run-tests/172/

When a block containing small files is being assembled, if the process is interrupted before the block is committed to Keep, arv-put will try to save its state before quitting but the cached collection's BlockManager will be in an inconsistent state (will still have the block in writable state, and without an owner because there are multiple files inside it) and will produce the stack trace described.
We now check for this case when trying to save the last checkpoint before quitting, and if it happens, print a warning message to the user and close the cache file.

So if I'm following along, the problem is that it is in repack_small_blocks() which creates a new, un-owned bufferblock into which it copies the other block data. If that gets interrupted, it escapes the method before calling _delete_bufferblock() so there is still an un-owned bufferblock. When it tries to flush again later, it runs into the un-owned bufferblock and fails because it isn't expecting it.

However the real problem is that KeyboardInterrupt can be thrown anywhere any time, which can leave the data structures in an inconsistent state, of which this bug is just one example. I am suspicious that trying to flush after catching ^C could also result in a silently corrupted checkpoint file. I think that when ^C (KeyboardInterrupt) is raised, it should not try to checkpoint.

Alternately, you could refactor it so that the upload happens on a different thread which doesn't get interrupted, and the main thread just sits in an idle loop. If the main thread gets a KeyboardInterrupt, it would then signal the uploader thread to perform a graceful shutdown.

Updates: a83e4de
Test run: https://ci.curoverse.com/job/developer-run-tests/175/

Changed so when catching a KeyboardInterrupt exception, stops the checkpointing thread and closes the cache file without trying to do the last update.

From our conversations on IRC, I think KeyboardInterrupt should skip checkpointing but shouldn't log anything. However, there should also be a default exception handler which which logs the exception and also skips checkpointing.

Updates at d2a202e

Added a specific exception class for this kind of errors, so it's not confused with other AttributeError bugs that could happen in the future.
Now state saving is not done when quitting from any kind of exception, reporting the stack trace to the logger, and not reporting anything when quitting from a KeyboardInterrupt (SystemExit(-2) in this case) or SIGINT.
Updated some tests that were relying on the previous behaviour.

From IRC: make a note around line L454 that because we catch signals, we expect a SystemExit, not KeyboardInterrupt.

Rest LGTM.