Skip to content

Commit 8c14224

Browse files
committed
refdb: make sure to remove packed refs first
This fixes part of the issue where, given a concurrent `git pack-refs`, a ref lookup could return an old, vestigial value from the packed file, as the valid loose one would have been deleted.
1 parent 171116e commit 8c14224

File tree

1 file changed

+24
-10
lines changed

1 file changed

+24
-10
lines changed

src/refdb_fs.c

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,7 @@ static int refdb_fs_backend__delete_tail(
14391439
{
14401440
refdb_fs_backend *backend = GIT_CONTAINER_OF(_backend, refdb_fs_backend, parent);
14411441
int error = 0, cmp = 0;
1442-
bool loose_deleted = 0;
1442+
bool packed_deleted = 0;
14431443

14441444
error = cmp_old_ref(&cmp, _backend, ref_name, old_id, old_target);
14451445
if (error < 0)
@@ -1451,26 +1451,40 @@ static int refdb_fs_backend__delete_tail(
14511451
goto cleanup;
14521452
}
14531453

1454-
/* If a loose reference exists, remove it from the filesystem */
1455-
if ((error = loose_delete(backend, ref_name)) < 0 && error != GIT_ENOTFOUND)
1454+
/*
1455+
* To ensure that an external observer will see either the current ref value
1456+
* (because the loose ref still exists), or a missing ref (after the packed-file is
1457+
* unlocked, there will be nothing left), we must ensure things happen in the
1458+
* following order:
1459+
*
1460+
* - the packed-ref file is locked and loaded, as well as a loose one, if it exists
1461+
* - we optimistically delete a packed ref, keeping track of whether it existed
1462+
* - we delete the loose ref, note that we have its .lock
1463+
* - the loose ref is "unlocked", then the packed-ref file is rewritten and unlocked
1464+
* - we should prune the path components if a loose ref was deleted
1465+
*
1466+
* Note that, because our packed backend doesn't expose its filesystem lock,
1467+
* we might not be able to guarantee that this is what actually happens (ie.
1468+
* as our current code never write packed-refs.lock, nothing stops observers
1469+
* from grabbing a "stale" value from there).
1470+
*/
1471+
if ((error = packed_delete(backend, ref_name)) < 0 && error != GIT_ENOTFOUND)
14561472
goto cleanup;
14571473

1458-
if (error == GIT_ENOTFOUND)
1459-
error = 0;
1460-
else if (error == 0)
1461-
loose_deleted = 1;
1474+
if (error == 0)
1475+
packed_deleted = 1;
14621476

1463-
if ((error = packed_delete(backend, ref_name)) < 0 && error != GIT_ENOTFOUND)
1477+
if ((error = loose_delete(backend, ref_name)) < 0 && error != GIT_ENOTFOUND)
14641478
goto cleanup;
14651479

14661480
if (error == GIT_ENOTFOUND) {
1467-
error = loose_deleted ? 0 : ref_error_notfound(ref_name);
1481+
error = packed_deleted ? 0 : ref_error_notfound(ref_name);
14681482
goto cleanup;
14691483
}
14701484

14711485
cleanup:
14721486
git_filebuf_cleanup(file);
1473-
if (loose_deleted)
1487+
if (error == 0)
14741488
refdb_fs_backend__prune_refs(backend, ref_name, "");
14751489
return error;
14761490
}

0 commit comments

Comments
 (0)