Skip to content

Commit 618dfa3

Browse files
committed
Fix race in dowloader.Cached()
Checking if an image is cached races with parallel downloads. Take the lock when validating the digest or the data file to ensure that we validate the cached when it is in consistent state. If an image is being downloaded, the check will block until the download completes. Signed-off-by: Nir Soffer <[email protected]>
1 parent 4fd2aaf commit 618dfa3

File tree

1 file changed

+22
-7
lines changed

1 file changed

+22
-7
lines changed

pkg/downloader/downloader.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -355,18 +355,33 @@ func Cached(remote string, opts ...Opt) (*Result, error) {
355355
if err != nil {
356356
return nil, err
357357
}
358+
359+
// Checking if data file exists is safe without locking.
358360
if _, err := os.Stat(shadData); err != nil {
359361
return nil, err
360362
}
361-
if _, err := os.Stat(shadDigest); err != nil {
362-
if err := validateCachedDigest(shadDigest, o.expectedDigest); err != nil {
363-
return nil, err
364-
}
365-
} else {
366-
if err := validateLocalFileDigest(shadData, o.expectedDigest); err != nil {
367-
return nil, err
363+
364+
// But validating the digest or the data file must take the lock to avoid races
365+
// with parallel downloads.
366+
if err := os.MkdirAll(shad, 0o700); err != nil {
367+
return nil, err
368+
}
369+
err = lockutil.WithDirLock(shad, func() error {
370+
if _, err := os.Stat(shadDigest); err != nil {
371+
if err := validateCachedDigest(shadDigest, o.expectedDigest); err != nil {
372+
return err
373+
}
374+
} else {
375+
if err := validateLocalFileDigest(shadData, o.expectedDigest); err != nil {
376+
return err
377+
}
368378
}
379+
return nil
380+
})
381+
if err != nil {
382+
return nil, err
369383
}
384+
370385
res := &Result{
371386
Status: StatusUsedCache,
372387
CachePath: shadData,

0 commit comments

Comments
 (0)