tree 5504fdf203fdf5d6c09d2debf68749417aa7ef94
parent aeb582a98374c094361cba1bd756dc6307432c42
author Jeff King <peff@peff.net> 1553160524 -0400
committer Junio C Hamano <gitster@pobox.com> 1553237569 +0900

refs/files-backend: handle packed transaction prepare failure

In files_transaction_prepare(), if we have to delete some refs, we use a
subordinate packed_transaction to do so. It's rare for that
sub-transaction's prepare step to fail, since we hold the packed-refs
lock. But if it does, we trigger a BUG() due to these steps:

  - we've attached the packed transaction to the files transaction as
    backend_data->packed_transaction

  - when the prepare step fails, the packed transaction cleans itself
    up, putting itself into the CLOSED state

  - the error value from preparing the packed transaction lets us know
    in files_transaction_prepare() that we should also clean up and
    return an error. We call files_transaction_cleanup(), which tries to
    abort backend_data->packed_transaction. Since it's already CLOSED,
    that triggers an assertion in ref_transaction_abort().

We can fix that by disconnecting the packed transaction from the outer
files transaction, and then free-ing (not aborting!) it ourselves.

A few other options/alternatives I considered:

  - we could just make it a noop to abort a CLOSED transaction. But that
    seems less safe, since clearly this code expects (and enforces) a
    particular set of state transitions.

  - we could have files_transaction_cleanup() selectively call abort()
    vs free() based on the state of the on the packed transaction.
    That's basically a more restricted version of the above, but also
    potentially unsafe.

  - instead of disconnecting backend_data->packed_transaction on error,
    we could wait to install it until we successfully prepare. That
    might make the flow a little simpler, but it introduces a hassle.
    Earlier parts of files_transaction_prepare() that encounter an error
    will jump to the cleanup label, and expect that cleaning up the
    outer transaction will clean up the packed transaction, too. We'd
    have to adjust those sites to clean up the packed transaction.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>