[PATCH 07/10] t1404: demonstrate two problems with reference transactions

From

Michael Haggerty <mhagger@alum.mit.edu>

Date

Aug 29, 2017, 08:20 UTC

Message-ID

<caaa44126f18869158872e5473e53478db780ba9.1503993268.git.mhagger@alum.mit.edu>

In-Reply-To

<cover.1503993268.git.mhagger@alum.mit.edu>

* 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>
---
The first of the new tests is rather involved; it uses two background
processes plus a foreground process that polls the value of a
reference. But despite sensitivity to timing, I think it should be
robust even in this broken state. Once the functionality being tested
is fixed, this test should never produce false positives, though
really bad timing (e.g., if it takes more than a second for
`update-ref` to get going) could still lead to false negatives.

 t/t1404-update-ref-errors.sh | 71 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index c34ece48f5..752f83c377 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -404,4 +404,75 @@ test_expect_success 'broken reference blocks indirect create' '
 	test_cmp expected output.err
 '
 
+test_expect_failure 'no bogus intermediate values during delete' '
+	prefix=refs/slow-transaction &&
+	# Set up a reference with differing loose and packed versions:
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# Now try to update the reference, but hold the `packed-refs` lock
+	# for a while to see what happens while the process is blocked:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	{
+		sleep 1 &&
+		rm -f .git/packed-refs.lock &
+	} &&
+	pid1=$! &&
+	{
+		# Note: the following command is intentionally run in the
+		# background. We extend the timeout so that `update-ref`
+		# tries to acquire the `packed-refs` lock longer than it
+		# takes the background process above to delete it:
+		git -c core.packedrefstimeout=2000 update-ref -d $prefix/foo &
+	} &&
+	pid2=$! &&
+	ok=true &&
+	while kill -0 $pid2 2>/dev/null
+	do
+		sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
+		case "$sha1" in
+		$D)
+			# This is OK; it just means that nothing has happened yet.
+			: ;;
+		undefined)
+			# This is OK; it means the deletion was successful.
+			: ;;
+		$C)
+			# This value should never be seen. Probably the loose
+			# reference has been deleted but the packed reference
+			# is still there:
+			echo "$prefix/foo incorrectly observed to be C" &&
+			break
+			;;
+		*)
+			# WTF?
+			echo "$prefix/foo unexpected value observed: $sha1" &&
+			break
+			;;
+		esac
+	done >out &&
+	wait $pid1 &&
+	wait $pid2 &&
+	test_must_be_empty out &&
+	test_must_fail git rev-parse --verify --quiet $prefix/foo
+'
+
+test_expect_failure 'delete fails cleanly if packed-refs file is locked' '
+	prefix=refs/locked-packed-refs &&
+	# Set up a reference with differing loose and packed versions:
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# Now try to delete it while the `packed-refs` lock is held:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	test_must_fail git update-ref -d $prefix/foo >out 2>err &&
+	git for-each-ref $prefix >actual &&
+	grep "Unable to create $Q.*packed-refs.lock$Q: File exists" err &&
+	test_cmp unchanged actual
+'
+
 test_done

-- 
2.14.1