housekeeping: Fix race condition in `addPackRefsInhibitor`

The `addPackRefsInhibitor` function allows users to add an inhibitor to
`git-pack-refs(1)`. In the function we check if `git-pack-refs(1)` is
currently/still running (in a loop) and allow the user to exit early via
context cancellation.

There is race condition here wherein we read from the
`state.packRefsDone` channel without a lock. This is a race condition,
since, the cleanup function of `tryRunningPackRefs` could also set
`state.packRefsDone` to nil concurrently. And reading from a nil channel
causes a panic.

We solve this race by using the local variable `packRefsDone`, which is
set after obtaining a lock. This way the local variable could be closed
but will not be `nil`, since only `state.packRefsDone` is set to `nil`.

1 files changed, 4 insertions, 1 deletions

diff --git a/internal/git/housekeeping/manager.go b/internal/git/housekeeping/manager.go
index 854d6c8ad..d73152182 100644
--- a/internal/git/housekeeping/manager.go
+++ b/internal/git/housekeeping/manager.go
@@ -144,7 +144,10 @@ func (s *repositoryStates) addPackRefsInhibitor(ctx context.Context, repoPath st
 		select {
 		case <-ctx.Done():
 			return false, nil, ctx.Err()
-		case <-state.packRefsDone:
+		case <-packRefsDone:
+			// We don't use state.packRefsDone, cause there is possibility that it is set
+			// to `nil` by the cleanup function after running `git-pack-refs(1)`.
+			//
 			// We obtain a lock and continue the loop here to avoid a race wherein another
 			// goroutine has invoked git-pack-refs(1). By continuing the loop and checking
 			// the value of packRefsDone, we can avoid that scenario.