Skip to content

Commit 6a2a773

Browse files
mhaggergitster
authored andcommitted
t1404: demonstrate two problems with reference transactions
Currently, a loose reference is deleted even before locking the `packed-refs` file, let alone deleting any packed version of the reference. This leads to two problems, demonstrated by two new tests: * While a reference is being deleted, other processes might see the old, packed value of the reference for a moment before the packed version is deleted. Normally this would be hard to observe, but we can prolong the window by locking the `packed-refs` file externally before running `update-ref`, then unlocking it before `update-ref`'s attempt to acquire the lock times out. * If the `packed-refs` file is locked so long that `update-ref` fails to lock it, then the reference can be left permanently in the incorrect state described in the previous point. In a moment, both problems will be fixed. Signed-off-by: Michael Haggerty <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1444bfe commit 6a2a773

File tree

1 file changed

+73
-0
lines changed

1 file changed

+73
-0
lines changed

t/t1404-update-ref-errors.sh

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,4 +404,77 @@ test_expect_success 'broken reference blocks indirect create' '
404404
test_cmp expected output.err
405405
'
406406

407+
test_expect_failure 'no bogus intermediate values during delete' '
408+
prefix=refs/slow-transaction &&
409+
# Set up a reference with differing loose and packed versions:
410+
git update-ref $prefix/foo $C &&
411+
git pack-refs --all &&
412+
git update-ref $prefix/foo $D &&
413+
git for-each-ref $prefix >unchanged &&
414+
# Now try to update the reference, but hold the `packed-refs` lock
415+
# for a while to see what happens while the process is blocked:
416+
: >.git/packed-refs.lock &&
417+
test_when_finished "rm -f .git/packed-refs.lock" &&
418+
{
419+
# Note: the following command is intentionally run in the
420+
# background. We increase the timeout so that `update-ref`
421+
# attempts to acquire the `packed-refs` lock for longer than
422+
# it takes for us to do the check then delete it:
423+
git -c core.packedrefstimeout=3000 update-ref -d $prefix/foo &
424+
} &&
425+
pid2=$! &&
426+
# Give update-ref plenty of time to get to the point where it tries
427+
# to lock packed-refs:
428+
sleep 1 &&
429+
# Make sure that update-ref did not complete despite the lock:
430+
kill -0 $pid2 &&
431+
# Verify that the reference still has its old value:
432+
sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
433+
case "$sha1" in
434+
$D)
435+
# This is what we hope for; it means that nothing
436+
# user-visible has changed yet.
437+
: ;;
438+
undefined)
439+
# This is not correct; it means the deletion has happened
440+
# already even though update-ref should not have been
441+
# able to acquire the lock yet.
442+
echo "$prefix/foo deleted prematurely" &&
443+
break
444+
;;
445+
$C)
446+
# This value should never be seen. Probably the loose
447+
# reference has been deleted but the packed reference
448+
# is still there:
449+
echo "$prefix/foo incorrectly observed to be C" &&
450+
break
451+
;;
452+
*)
453+
# WTF?
454+
echo "unexpected value observed for $prefix/foo: $sha1" &&
455+
break
456+
;;
457+
esac >out &&
458+
rm -f .git/packed-refs.lock &&
459+
wait $pid2 &&
460+
test_must_be_empty out &&
461+
test_must_fail git rev-parse --verify --quiet $prefix/foo
462+
'
463+
464+
test_expect_failure 'delete fails cleanly if packed-refs file is locked' '
465+
prefix=refs/locked-packed-refs &&
466+
# Set up a reference with differing loose and packed versions:
467+
git update-ref $prefix/foo $C &&
468+
git pack-refs --all &&
469+
git update-ref $prefix/foo $D &&
470+
git for-each-ref $prefix >unchanged &&
471+
# Now try to delete it while the `packed-refs` lock is held:
472+
: >.git/packed-refs.lock &&
473+
test_when_finished "rm -f .git/packed-refs.lock" &&
474+
test_must_fail git update-ref -d $prefix/foo >out 2>err &&
475+
git for-each-ref $prefix >actual &&
476+
test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File exists" err &&
477+
test_cmp unchanged actual
478+
'
479+
407480
test_done

0 commit comments

Comments
 (0)