This repository was archived by the owner on Aug 14, 2020. It is now read-only.
lib/internal/backend/repository: check if closer is nil before closing#162
Merged
iaguis merged 1 commit intoappc:masterfrom May 26, 2016
Merged
lib/internal/backend/repository: check if closer is nil before closing#162iaguis merged 1 commit intoappc:masterfrom
iaguis merged 1 commit intoappc:masterfrom
Conversation
If any layers being fetched encountered an error, that won't be caught by the main goroutine until after it's attempted to close all of the HTTP bodies, but if there was an error the body could be nil. The bodies should be closed before checking for errors, as there may be some valid connections, but each body should only be closed if it's not nil. This commit adds that nil check.
| @@ -125,7 +125,9 @@ func (rb *RepositoryBackend) buildACIV2(layerIDs []string, dockerURL *types.Pars | |||
| return nil, nil, err | |||
| } | |||
| for _, closer := range closers { | |||
Contributor
There was a problem hiding this comment.
i) closers can just be []io.Closer
ii) why not only append to closers if getLayerV2 returns a nil err?
Member
Author
There was a problem hiding this comment.
For ii, it's because I was to be very explicit that layerFiles must be the same length as layerIDs (which is clear in layerFiles := make([]*os.File, len(layerIDs))), and closers is just following the same pattern.
Contributor
|
looks okay but I'll leave it to @iaguis |
Member
|
LGTM |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If any layers being fetched encountered an error, that won't be caught
by the main goroutine until after it's attempted to close all of the
HTTP bodies, but if there was an error the body could be nil. The bodies
should be closed before checking for errors, as there may be some valid
connections, but each body should only be closed if it's not nil.
This commit adds that nil check.
Fixes (at least when I run the test locally) https://semaphoreci.com/coreos/rkt/branches/pull-request-2691/builds/3
cc @iaguis