Skip to content

Commit 2e08c89

Browse files
committed
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
2 parents 3feaacb + d3322eb commit 2e08c89

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-1
lines changed

refs/files-backend.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2701,18 +2701,32 @@ static int files_transaction_prepare(struct ref_store *ref_store,
27012701
if (is_packed_transaction_needed(refs->packed_ref_store,
27022702
packed_transaction)) {
27032703
ret = ref_transaction_prepare(packed_transaction, err);
2704+
/*
2705+
* A failure during the prepare step will abort
2706+
* itself, but not free. Do that now, and disconnect
2707+
* from the files_transaction so it does not try to
2708+
* abort us when we hit the cleanup code below.
2709+
*/
2710+
if (ret) {
2711+
ref_transaction_free(packed_transaction);
2712+
backend_data->packed_transaction = NULL;
2713+
}
27042714
} else {
27052715
/*
27062716
* We can skip rewriting the `packed-refs`
27072717
* file. But we do need to leave it locked, so
27082718
* that somebody else doesn't pack a reference
27092719
* that we are trying to delete.
2720+
*
2721+
* We need to disconnect our transaction from
2722+
* backend_data, since the abort (whether successful or
2723+
* not) will free it.
27102724
*/
2725+
backend_data->packed_transaction = NULL;
27112726
if (ref_transaction_abort(packed_transaction, err)) {
27122727
ret = TRANSACTION_GENERIC_ERROR;
27132728
goto cleanup;
27142729
}
2715-
backend_data->packed_transaction = NULL;
27162730
}
27172731
}
27182732

t/t1404-update-ref-errors.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,4 +618,20 @@ test_expect_success 'delete fails cleanly if packed-refs file is locked' '
618618
test_cmp unchanged actual
619619
'
620620

621+
test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
622+
# Setup and expectations are similar to the test above.
623+
prefix=refs/failed-packed-refs &&
624+
git update-ref $prefix/foo $C &&
625+
git pack-refs --all &&
626+
git update-ref $prefix/foo $D &&
627+
git for-each-ref $prefix >unchanged &&
628+
# This should not happen in practice, but it is an easy way to get a
629+
# reliable error (we open with create_tempfile(), which uses O_EXCL).
630+
: >.git/packed-refs.new &&
631+
test_when_finished "rm -f .git/packed-refs.new" &&
632+
test_must_fail git update-ref -d $prefix/foo &&
633+
git for-each-ref $prefix >actual &&
634+
test_cmp unchanged actual
635+
'
636+
621637
test_done

0 commit comments

Comments
 (0)