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 <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Sep 8, 2017 at 15:51 UTC 6a2a7736d867dac39f2c54833fd0a65107c7cc5b
1 file changed +73
t/t1404-update-ref-errors.sh
+73
@@ -404,4 +404,77 @@ test_expect_success 'broken reference blocks indirect create' '
404 test_cmp expected output.err
405 '
406
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 +
480 test_done