Skip to content

catfile: Fix race condition in `CatfileObject`

Karthik Nayak requested to merge 5729-flaky-test-testcatfileobject-spawning_two_pipes_fails into master

There is a race condition in the catfile package's CatfileObject function. The race condition can be easily triggered by adding a small sleep in the current code:

--- a/internal/git/gitpipe/catfile_object.go
+++ b/internal/git/gitpipe/catfile_object.go
@@ -49,6 +50,9 @@ func CatfileObject(
        go func() {
                defer func() {
                        close(requestChan)
+                       time.Sleep(1 * time.Millisecond)
                        if atomic.AddInt32(&queueRefcount, -1) == 0 {
                                queueCleanup()
                        }

This happens because we close the channel first and only then call the queue cleanup function. Users of CatfileObject receive an iterator which they iterate over, the iterator's Next() function returns false when there are no more items to iterate over.

So by closing the channel, the Next() function would return false. So a user could potentially try to reuse the objectQueue for another iteration. But the queue's cleanup potentially might not have happened. Fix this racy condition by doing the cleanup before closing the channel. The same case can be found around the resultChan too, this patch fixes both the regions.

Closes #5729 (closed)

Merge request reports