diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-06-03 08:21:12 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-06-03 08:49:16 +0300 |
| commit | 49ccf030bbfc9b9d023e4edfbe2c10e7df2d2671 (patch) | |
| tree | 450a5b00db83776903cdee9d0f29565437c28c70 | |
| parent | 18f838be8f80a550b5febbad025d9c61fe0ae8b7 (diff) | |
catfile: Keep request queue dirty if reading object info fails
When reading the object info fails we're currently returning the request
queue back to the state where it pretends to not be reading an object
anymore. This means that it is now not considered dirty anymore and may
be used for additional requests, or even be returned to the catfile
cache. This is dangerous though: we do not know why reading the object
info has failed, and chances are high that the error is not recoverable.
And in that case we certainly don't want to return the catfile process
to the cache.
Fix this isssue by not unsetting `isReadingObject` when `ReadObject()`
fails. The only exception is when we got a `NotFound` error, which is a
graceful failure and doesn't dirty the git-cat-file(1) process.
| -rw-r--r-- | internal/git/catfile/request_queue.go | 13 |
1 files changed, 12 insertions, 1 deletions
diff --git a/internal/git/catfile/request_queue.go b/internal/git/catfile/request_queue.go index e6090990c..5b1558efa 100644 --- a/internal/git/catfile/request_queue.go +++ b/internal/git/catfile/request_queue.go @@ -128,7 +128,18 @@ func (q *requestQueue) ReadObject() (*Object, error) { objectInfo, err := q.readInfo() if err != nil { - atomic.StoreInt32(&q.isReadingObject, 0) + // In the general case we cannot know why reading the object's info has failed. And + // given that git-cat-file(1) is stateful, we cannot say whether it's safe to + // continue reading from it now or whether we need to keep the queue dirty instead. + // So we keep `isReadingObject == 0` in the general case so that it continues to + // stay dirty. + // + // One known exception is when we've got a NotFoundError: this is a graceful failure + // and we can continue reading from the process. + if IsNotFound(err) { + atomic.StoreInt32(&q.isReadingObject, 0) + } + return nil, err } q.trace.recordRequest(objectInfo.Type) |