Browse code
Pull: only close temporary file once
Close could be called twice on a temporary download file, which could
have bad side effects.
This fixes the problem by setting to ld.tmpFile to nil when the download
completes sucessfully. Then the call to ld.Close will have no effect,
and only the download manager will close the temporary file when it's
done extracting the layer from it. ld.Close will be responsible for
closing the file if we hit the retry limit and there is still a partial
download present.
Fixes #21675
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Aaron Lehmann authored on 2016/03/31 11:26:12Showing 2 changed files
| ... | ... |
@@ -330,7 +330,20 @@ func (ld *v1LayerDescriptor) Download(ctx context.Context, progressOutput progre |
| 330 | 330 |
logrus.Debugf("Downloaded %s to tempfile %s", ld.ID(), ld.tmpFile.Name())
|
| 331 | 331 |
|
| 332 | 332 |
ld.tmpFile.Seek(0, 0) |
| 333 |
- return ld.tmpFile, ld.layerSize, nil |
|
| 333 |
+ |
|
| 334 |
+ // hand off the temporary file to the download manager, so it will only |
|
| 335 |
+ // be closed once |
|
| 336 |
+ tmpFile := ld.tmpFile |
|
| 337 |
+ ld.tmpFile = nil |
|
| 338 |
+ |
|
| 339 |
+ return ioutils.NewReadCloserWrapper(tmpFile, func() error {
|
|
| 340 |
+ tmpFile.Close() |
|
| 341 |
+ err := os.RemoveAll(tmpFile.Name()) |
|
| 342 |
+ if err != nil {
|
|
| 343 |
+ logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name())
|
|
| 344 |
+ } |
|
| 345 |
+ return err |
|
| 346 |
+ }), ld.layerSize, nil |
|
| 334 | 347 |
} |
| 335 | 348 |
|
| 336 | 349 |
func (ld *v1LayerDescriptor) Close() {
|
| ... | ... |
@@ -278,7 +278,19 @@ func (ld *v2LayerDescriptor) Download(ctx context.Context, progressOutput progre |
| 278 | 278 |
ld.verifier = nil |
| 279 | 279 |
return nil, 0, xfer.DoNotRetry{Err: err}
|
| 280 | 280 |
} |
| 281 |
- return tmpFile, size, nil |
|
| 281 |
+ |
|
| 282 |
+ // hand off the temporary file to the download manager, so it will only |
|
| 283 |
+ // be closed once |
|
| 284 |
+ ld.tmpFile = nil |
|
| 285 |
+ |
|
| 286 |
+ return ioutils.NewReadCloserWrapper(tmpFile, func() error {
|
|
| 287 |
+ tmpFile.Close() |
|
| 288 |
+ err := os.RemoveAll(tmpFile.Name()) |
|
| 289 |
+ if err != nil {
|
|
| 290 |
+ logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name())
|
|
| 291 |
+ } |
|
| 292 |
+ return err |
|
| 293 |
+ }), size, nil |
|
| 282 | 294 |
} |
| 283 | 295 |
|
| 284 | 296 |
func (ld *v2LayerDescriptor) Close() {
|