diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-09-23 16:52:41 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-10-06 13:39:32 +0300 |
commit | c01aed4046e1fc06a4487b505be45970814f21ac (patch) | |
tree | 26ba3a386206d00fb666b0a077da1197f463f7a2 /internal/git | |
parent | 74e745003b1a83e1f31a79e618715024c45f12c1 (diff) |
catfile: Make `close()` a private function
We do not allow callers outside of the catfile package to call `Close()`
on returned processes given that we want to be the only ones which
manage the lifetime of potentially cached processes.
Make the function private to better highlight this. This allows us to
embed it into the reader interfaces without exposing close to outside
callers such that we can easily access the `close()` function on
interfaces internally.
Diffstat (limited to 'internal/git')
-rw-r--r-- | internal/git/catfile/batch.go | 8 | ||||
-rw-r--r-- | internal/git/catfile/cache.go | 10 | ||||
-rw-r--r-- | internal/git/catfile/cache_test.go | 2 | ||||
-rw-r--r-- | internal/git/catfile/object_info_reader.go | 4 | ||||
-rw-r--r-- | internal/git/catfile/object_reader.go | 4 |
5 files changed, 14 insertions, 14 deletions
diff --git a/internal/git/catfile/batch.go b/internal/git/catfile/batch.go index 5c11ad015..fa1dd8440 100644 --- a/internal/git/catfile/batch.go +++ b/internal/git/catfile/batch.go @@ -54,7 +54,7 @@ func newBatch( // If creation of the ObjectInfoReader fails, then we do not want to leak the // ObjectReader process. if returnedErr != nil { - objectReader.Close() + objectReader.close() } }() @@ -68,7 +68,7 @@ func newBatch( // Close closes the writers for objectInfoReader and objectReader. This is only used for cached // Batches -func (c *batch) Close() { +func (c *batch) close() { c.closedMutex.Lock() defer c.closedMutex.Unlock() @@ -77,8 +77,8 @@ func (c *batch) Close() { } c.closed = true - c.objectReader.Close() - c.objectInfoReader.Close() + c.objectReader.close() + c.objectInfoReader.close() } func (c *batch) isClosed() bool { diff --git a/internal/git/catfile/cache.go b/internal/git/catfile/cache.go index 3c3e4ca85..ac63f8434 100644 --- a/internal/git/catfile/cache.go +++ b/internal/git/catfile/cache.go @@ -38,7 +38,7 @@ type Cache interface { type cacheable interface { isClosed() bool isDirty() bool - Close() + close() } // ProcessCache entries always get added to the back of the list. If the @@ -209,7 +209,7 @@ func (c *ProcessCache) getOrCreateProcess( // We have not found any cached process, so we need to create a new one. In this // case, we need to detach the process from the current context such that it does // not get killed when the current context is done. Note that while we explicitly - // `Close()` processes in case this function fails, we must have a cancellable + // `close()` processes in case this function fails, we must have a cancellable // context or otherwise our `command` package will panic. ctx, cancel = context.WithCancel(context.Background()) defer func() { @@ -232,7 +232,7 @@ func (c *ProcessCache) getOrCreateProcess( // If we somehow fail after creating a new Batch process, then we want to kill // spawned processes right away. if returnedErr != nil { - batch.Close() + batch.close() } }() @@ -280,7 +280,7 @@ func (c *ProcessCache) returnWhenDone(done <-chan struct{}, s *stack, cacheKey k if value.isDirty() { cancel() c.catfileCacheCounter.WithLabelValues("dirty").Inc() - value.Close() + value.close() return } @@ -413,7 +413,7 @@ func (s *stack) delete(i int, wantClose bool) { ent := s.entries[i] if wantClose { - ent.value.Close() + ent.value.close() ent.cancel() } diff --git a/internal/git/catfile/cache_test.go b/internal/git/catfile/cache_test.go index 5f654f67e..82c96b532 100644 --- a/internal/git/catfile/cache_test.go +++ b/internal/git/catfile/cache_test.go @@ -327,7 +327,7 @@ func TestCache_BatchProcess(t *testing.T) { // Closed processes naturally cannot be reused anymore and thus shouldn't ever get // cached. - batch.Close() + batch.close() // Cancel the context such that the process will be considered for return to the // cache and wait for the cache to collect it. diff --git a/internal/git/catfile/object_info_reader.go b/internal/git/catfile/object_info_reader.go index 75d38c01e..e10c13a01 100644 --- a/internal/git/catfile/object_info_reader.go +++ b/internal/git/catfile/object_info_reader.go @@ -113,14 +113,14 @@ func newObjectInfoReader( go func() { <-ctx.Done() // This is crucial to prevent leaking file descriptors. - objectInfoReader.Close() + objectInfoReader.close() span.Finish() }() return objectInfoReader, nil } -func (o *objectInfoReader) Close() { +func (o *objectInfoReader) close() { _ = o.cmd.Wait() } diff --git a/internal/git/catfile/object_reader.go b/internal/git/catfile/object_reader.go index d16a2d09d..14edf344a 100644 --- a/internal/git/catfile/object_reader.go +++ b/internal/git/catfile/object_reader.go @@ -76,14 +76,14 @@ func newObjectReader( } go func() { <-ctx.Done() - objectReader.Close() + objectReader.close() span.Finish() }() return objectReader, nil } -func (o *objectReader) Close() { +func (o *objectReader) close() { _ = o.cmd.Wait() } |