diff options
author | Junio C Hamano <gitster@pobox.com> | 2019-04-16 13:28:11 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-04-16 13:28:11 +0300 |
commit | 2e08c892a7037ce2c7dfe30134eb54a2825bf0be (patch) | |
tree | b2fd76651d0f0bb17e788da15612f4640f30f189 | |
parent | 3feaacbaa15f6437a1767935a5ff04fdef0e86e3 (diff) | |
parent | d3322eb28b142a893fbc03142724bb54e95990a6 (diff) |
Merge branch 'jk/refs-double-abort'
A corner case bug in the refs API has been corrected.
* jk/refs-double-abort:
refs/files-backend: don't look at an aborted transaction
refs/files-backend: handle packed transaction prepare failure
-rw-r--r-- | refs/files-backend.c | 16 | ||||
-rwxr-xr-x | t/t1404-update-ref-errors.sh | 16 |
2 files changed, 31 insertions, 1 deletions
diff --git a/refs/files-backend.c b/refs/files-backend.c index 5848f32ef8..63e55e6773 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2701,18 +2701,32 @@ static int files_transaction_prepare(struct ref_store *ref_store, if (is_packed_transaction_needed(refs->packed_ref_store, packed_transaction)) { ret = ref_transaction_prepare(packed_transaction, err); + /* + * A failure during the prepare step will abort + * itself, but not free. Do that now, and disconnect + * from the files_transaction so it does not try to + * abort us when we hit the cleanup code below. + */ + if (ret) { + ref_transaction_free(packed_transaction); + backend_data->packed_transaction = NULL; + } } else { /* * We can skip rewriting the `packed-refs` * file. But we do need to leave it locked, so * that somebody else doesn't pack a reference * that we are trying to delete. + * + * We need to disconnect our transaction from + * backend_data, since the abort (whether successful or + * not) will free it. */ + backend_data->packed_transaction = NULL; if (ref_transaction_abort(packed_transaction, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } - backend_data->packed_transaction = NULL; } } diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh index 6b6a8e2292..970c5c36b9 100755 --- a/t/t1404-update-ref-errors.sh +++ b/t/t1404-update-ref-errors.sh @@ -618,4 +618,20 @@ test_expect_success 'delete fails cleanly if packed-refs file is locked' ' test_cmp unchanged actual ' +test_expect_success 'delete fails cleanly if packed-refs.new write fails' ' + # Setup and expectations are similar to the test above. + prefix=refs/failed-packed-refs && + git update-ref $prefix/foo $C && + git pack-refs --all && + git update-ref $prefix/foo $D && + git for-each-ref $prefix >unchanged && + # This should not happen in practice, but it is an easy way to get a + # reliable error (we open with create_tempfile(), which uses O_EXCL). + : >.git/packed-refs.new && + test_when_finished "rm -f .git/packed-refs.new" && + test_must_fail git update-ref -d $prefix/foo && + git for-each-ref $prefix >actual && + test_cmp unchanged actual +' + test_done |