vendor: update c/storage #2844
vendor: update c/storage #2844
Conversation
There was a problem hiding this comment.
I’m afraid I’m behind on the c/storage PR, but, how does this work?
AFAICS this means that chunkedDiffer.tarSplit is closed by the time this function returns.
But, just below, PrepareStagedLayer ultimately calls chunkedDiffer.ApplyDiff, which copies differ.tarSplit to out.TarSplit. This function stores out to diffOutputs and returns, closing tarSplit.
Later, createNewLayer calls ApplyStagedLayer, and that, in applyDiffFromStagingDirectory, tries to read an already-closed file.
What am I missing?
There was a problem hiding this comment.
Note to self: this will be correct with containers/storage#2323 .
giuseppe
force-pushed
the
update-c-storage-29-apr-2025
branch
2 times, most recently
from
ce9b3b2 to
0382a71
Compare
|
rebased |
| @@ -421,7 +421,7 @@ func (s *storageImageDestination) PutBlobPartial(ctx context.Context, chunkAcces | |||
| } | |||
| }() | |||
|
|
|||
| differ, err := chunked.GetDiffer(ctx, s.imageRef.transport.store, srcInfo.Digest, srcInfo.Size, srcInfo.Annotations, &fetcher) | |||
| differ, err := chunked.NewDiffer(ctx, s.imageRef.transport.store, srcInfo.Digest, srcInfo.Size, srcInfo.Annotations, &fetcher) | |||
There was a problem hiding this comment.
The part that ensured differ.Close was called (and that motivated breaking the API to s/Get/New/ here) got lost?!
There was a problem hiding this comment.
sorry I missed the error path on s.imageRef.transport.store.PrepareStagedLayer(), because later on CleanupStagedLayer takes care of it.
Do you prefer to have the differ.Close() just there or through a new defer function?
There was a problem hiding this comment.
containers/storage#2323 now documents on NewDiffer that
The caller must call Close() on the returned Differ.
so I’d prefer doing exactly that — defer seems easiest.
Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
update-c-storage-29-apr-2025
branch
from
0382a71 to
3c26936
Compare
follow-up for containers/storage#2312