catfile: Fix race condition in `CatfileObject`

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:

```go
--- 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.

Also check for `it.Err()` in the test.

2 files changed, 3 insertions, 2 deletions

diff --git a/internal/git/gitpipe/catfile_object.go b/internal/git/gitpipe/catfile_object.go
index b809923d9..77c7fecd4 100644
--- a/internal/git/gitpipe/catfile_object.go
+++ b/internal/git/gitpipe/catfile_object.go
@@ -48,10 +48,10 @@ func CatfileObject(
 	requestChan := make(chan catfileObjectRequest, 32)
 	go func() {
 		defer func() {
-			close(requestChan)
 			if atomic.AddInt32(&queueRefcount, -1) == 0 {
 				queueCleanup()
 			}
+			close(requestChan)
 		}()
 
 		sendRequest := func(request catfileObjectRequest) bool {
@@ -115,10 +115,10 @@ func CatfileObject(
 	resultChan := make(chan CatfileObjectResult)
 	go func() {
 		defer func() {
-			close(resultChan)
 			if atomic.AddInt32(&queueRefcount, -1) == 0 {
 				queueCleanup()
 			}
+			close(resultChan)
 		}()
 
 		sendResult := func(result CatfileObjectResult) bool {
diff --git a/internal/git/gitpipe/catfile_object_test.go b/internal/git/gitpipe/catfile_object_test.go
index b53d86aea..59872be13 100644
--- a/internal/git/gitpipe/catfile_object_test.go
+++ b/internal/git/gitpipe/catfile_object_test.go
@@ -243,6 +243,7 @@ func TestCatfileObject(t *testing.T) {
 		_, err = io.Copy(io.Discard, it.Result())
 		require.NoError(t, err)
 		require.False(t, it.Next())
+		require.NoError(t, it.Err())
 
 		// Which means that the queue should now be unused, so we can again use it.
 		_, err = CatfileObject(ctx, objectReader, NewRevisionIterator(ctx, input))