From ca8158ba7f49a8e3cde576bf89904b78b0096f87 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 15:17:11 +0200 Subject: [PATCH 1/8] overlay: fix comments Signed-off-by: Giuseppe Scrivano --- drivers/overlay/overlay.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index f26cf695bd..b3b551b7e7 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -876,9 +876,9 @@ func (d *Driver) Metadata(id string) (map[string]string, error) { return metadata, nil } -// Cleanup any state created by overlay which should be cleaned when daemon -// is being shutdown. For now, we just have to unmount the bind mounted -// we had created. +// Cleanup any state created by overlay which should be cleaned when +// the storage is being shutdown. The only state created by the driver +// is the bind mount on the home directory. func (d *Driver) Cleanup() error { _ = os.RemoveAll(filepath.Join(d.home, stagingDir)) return mount.Unmount(d.home) @@ -2050,8 +2050,8 @@ func supportsDataOnlyLayersCached(home, runhome string) (bool, error) { return supportsDataOnly, err } -// ApplyDiff applies the changes in the new layer using the specified function -func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.ApplyDiffWithDifferOpts, differ graphdriver.Differ) (output graphdriver.DriverWithDifferOutput, err error) { +// ApplyDiffWithDiffer applies the changes in the new layer using the specified function +func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.ApplyDiffWithDifferOpts, differ graphdriver.Differ) (output graphdriver.DriverWithDifferOutput, errRet error) { var idMappings *idtools.IDMappings if options != nil { idMappings = options.Mappings From 1e4399251559ead4d6edcc18e0ae91c2c7496329 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 15:25:10 +0200 Subject: [PATCH 2/8] lockfile: fix comment Signed-off-by: Giuseppe Scrivano --- pkg/lockfile/lockfile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lockfile/lockfile.go b/pkg/lockfile/lockfile.go index 5dd6741086..19694be3bc 100644 --- a/pkg/lockfile/lockfile.go +++ b/pkg/lockfile/lockfile.go @@ -133,7 +133,7 @@ func (l *LockFile) Lock() { } } -// LockRead locks the lockfile as a reader. +// RLock locks the lockfile as a reader. func (l *LockFile) RLock() { l.lock(readLock) } From 26e97fbe3d9989de56540e46649fecb9474056a3 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 17:23:54 +0200 Subject: [PATCH 3/8] store: extend ApplyStagedLayer to use existing layers Signed-off-by: Giuseppe Scrivano --- store.go | 57 +++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/store.go b/store.go index 253218e5ad..928df909c2 100644 --- a/store.go +++ b/store.go @@ -1445,20 +1445,8 @@ func (s *store) canUseShifting(uidmap, gidmap []idtools.IDMap) bool { return true } -func (s *store) putLayer(id, parent string, names []string, mountLabel string, writeable bool, lOptions *LayerOptions, diff io.Reader, slo *stagedLayerOptions) (*Layer, int64, error) { - rlstore, rlstores, err := s.bothLayerStoreKinds() - if err != nil { - return nil, -1, err - } - if err := rlstore.startWriting(); err != nil { - return nil, -1, err - } - defer rlstore.stopWriting() - if err := s.containerStore.startWriting(); err != nil { - return nil, -1, err - } - defer s.containerStore.stopWriting() - +// putLayer requires the rlstore, rlstores, as well as s.containerStore (even if not an argument to this function) to be locked for write. +func (s *store) putLayer(rlstore rwLayerStore, rlstores []roLayerStore, id, parent string, names []string, mountLabel string, writeable bool, lOptions *LayerOptions, diff io.Reader, slo *stagedLayerOptions) (*Layer, int64, error) { var parentLayer *Layer var options LayerOptions if lOptions != nil { @@ -1537,7 +1525,19 @@ func (s *store) putLayer(id, parent string, names []string, mountLabel string, w } func (s *store) PutLayer(id, parent string, names []string, mountLabel string, writeable bool, lOptions *LayerOptions, diff io.Reader) (*Layer, int64, error) { - return s.putLayer(id, parent, names, mountLabel, writeable, lOptions, diff, nil) + rlstore, rlstores, err := s.bothLayerStoreKinds() + if err != nil { + return nil, -1, err + } + if err := rlstore.startWriting(); err != nil { + return nil, -1, err + } + defer rlstore.stopWriting() + if err := s.containerStore.startWriting(); err != nil { + return nil, -1, err + } + defer s.containerStore.stopWriting() + return s.putLayer(rlstore, rlstores, id, parent, names, mountLabel, writeable, lOptions, diff, nil) } func (s *store) CreateLayer(id, parent string, names []string, mountLabel string, writeable bool, options *LayerOptions) (*Layer, error) { @@ -3016,12 +3016,35 @@ func (s *store) ApplyDiffFromStagingDirectory(to, stagingDirectory string, diffO } func (s *store) ApplyStagedLayer(args ApplyStagedLayerOptions) (*Layer, error) { + rlstore, rlstores, err := s.bothLayerStoreKinds() + if err != nil { + return nil, err + } + if err := rlstore.startWriting(); err != nil { + return nil, err + } + defer rlstore.stopWriting() + + layer, err := rlstore.Get(args.ID) + if err != nil && !errors.Is(err, ErrLayerUnknown) { + return layer, err + } + if err == nil { + return layer, rlstore.applyDiffFromStagingDirectory(args.ID, args.DiffOutput, args.DiffOptions) + } + + // if the layer doesn't exist yet, try to create it. + + if err := s.containerStore.startWriting(); err != nil { + return nil, err + } + defer s.containerStore.stopWriting() + slo := stagedLayerOptions{ DiffOutput: args.DiffOutput, DiffOptions: args.DiffOptions, } - - layer, _, err := s.putLayer(args.ID, args.ParentLayer, args.Names, args.MountLabel, args.Writeable, args.LayerOptions, nil, &slo) + layer, _, err = s.putLayer(rlstore, rlstores, args.ID, args.ParentLayer, args.Names, args.MountLabel, args.Writeable, args.LayerOptions, nil, &slo) return layer, err } From e6d23590dad0df251413cca5341788a6b7dbd14b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 12:57:05 +0200 Subject: [PATCH 4/8] cmd: replace usage of deprecated functions Signed-off-by: Giuseppe Scrivano --- cmd/containers-storage/diff.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/containers-storage/diff.go b/cmd/containers-storage/diff.go index 2811764d52..09f8b61f04 100644 --- a/cmd/containers-storage/diff.go +++ b/cmd/containers-storage/diff.go @@ -185,8 +185,14 @@ func applyDiffUsingStagingDirectory(flags *mflag.FlagSet, action string, m stora if err != nil { return 1, err } - if err := m.ApplyDiffFromStagingDirectory(layer, out.Target, out, &options); err != nil { - m.CleanupStagingDirectory(out.Target) + + applyStagedLayerArgs := storage.ApplyStagedLayerOptions{ + ID: layer, + DiffOutput: out, + DiffOptions: &options, + } + if _, err := m.ApplyStagedLayer(applyStagedLayerArgs); err != nil { + m.CleanupStagedLayer(out) return 1, err } return 0, nil From eb53d591a216f205bde214c3c5950e7e2e794229 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 12:58:26 +0200 Subject: [PATCH 5/8] store: drop deprecated functions the callers in c/image were already replaced, so simplify the store API and drop the functions. Signed-off-by: Giuseppe Scrivano --- deprecated.go | 2 -- store.go | 34 +++------------------------------- 2 files changed, 3 insertions(+), 33 deletions(-) diff --git a/deprecated.go b/deprecated.go index 04972d8388..76ae6328bf 100644 --- a/deprecated.go +++ b/deprecated.go @@ -208,8 +208,6 @@ type LayerStore interface { ParentOwners(id string) (uids, gids []int, err error) ApplyDiff(to string, diff io.Reader) (int64, error) ApplyDiffWithDiffer(to string, options *drivers.ApplyDiffOpts, differ drivers.Differ) (*drivers.DriverWithDifferOutput, error) - CleanupStagingDirectory(stagingDirectory string) error - ApplyDiffFromStagingDirectory(id, stagingDirectory string, diffOutput *drivers.DriverWithDifferOutput, options *drivers.ApplyDiffOpts) error DifferTarget(id string) (string, error) LoadLocked() error PutAdditionalLayer(id string, parentLayer *Layer, names []string, aLayer drivers.AdditionalLayer) (layer *Layer, err error) diff --git a/store.go b/store.go index 928df909c2..3c593c4009 100644 --- a/store.go +++ b/store.go @@ -330,17 +330,9 @@ type Store interface { // successfully applied with ApplyDiffFromStagingDirectory. ApplyDiffWithDiffer(to string, options *drivers.ApplyDiffWithDifferOpts, differ drivers.Differ) (*drivers.DriverWithDifferOutput, error) - // ApplyDiffFromStagingDirectory uses stagingDirectory to create the diff. - // Deprecated: it will be removed soon. Use ApplyStagedLayer instead. - ApplyDiffFromStagingDirectory(to, stagingDirectory string, diffOutput *drivers.DriverWithDifferOutput, options *drivers.ApplyDiffWithDifferOpts) error - - // CleanupStagingDirectory cleanups the staging directory. It can be used to cleanup the staging directory on errors - // Deprecated: it will be removed soon. Use CleanupStagedLayer instead. - CleanupStagingDirectory(stagingDirectory string) error - - // ApplyStagedLayer combines the functions of CreateLayer and ApplyDiffFromStagingDirectory, - // marking the layer for automatic removal if applying the diff fails - // for any reason. + // ApplyStagedLayer combines the functions of creating a layer and using the staging + // directory to populate it. + // It marks the layer for automatic removal if applying the diff fails for any reason. ApplyStagedLayer(args ApplyStagedLayerOptions) (*Layer, error) // CleanupStagedLayer cleanups the staging directory. It can be used to cleanup the staging directory on errors @@ -3002,19 +2994,6 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro return nil, ErrLayerUnknown } -func (s *store) ApplyDiffFromStagingDirectory(to, stagingDirectory string, diffOutput *drivers.DriverWithDifferOutput, options *drivers.ApplyDiffWithDifferOpts) error { - if stagingDirectory != diffOutput.Target { - return fmt.Errorf("invalid value for staging directory, it must be the same as the differ target directory") - } - _, err := writeToLayerStore(s, func(rlstore rwLayerStore) (struct{}, error) { - if !rlstore.Exists(to) { - return struct{}{}, ErrLayerUnknown - } - return struct{}{}, rlstore.applyDiffFromStagingDirectory(to, diffOutput, options) - }) - return err -} - func (s *store) ApplyStagedLayer(args ApplyStagedLayerOptions) (*Layer, error) { rlstore, rlstores, err := s.bothLayerStoreKinds() if err != nil { @@ -3048,13 +3027,6 @@ func (s *store) ApplyStagedLayer(args ApplyStagedLayerOptions) (*Layer, error) { return layer, err } -func (s *store) CleanupStagingDirectory(stagingDirectory string) error { - _, err := writeToLayerStore(s, func(rlstore rwLayerStore) (struct{}, error) { - return struct{}{}, rlstore.CleanupStagingDirectory(stagingDirectory) - }) - return err -} - func (s *store) CleanupStagedLayer(diffOutput *drivers.DriverWithDifferOutput) error { _, err := writeToLayerStore(s, func(rlstore rwLayerStore) (struct{}, error) { return struct{}{}, rlstore.CleanupStagingDirectory(diffOutput.Target) From 76f8994286c4e5a7d596a9ad96a9a38a0bc76c52 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 14:11:39 +0200 Subject: [PATCH 6/8] overlay: use a sub-directory for the staging path this is a preparatory patch to allow storing a lock file for each staging directory. Signed-off-by: Giuseppe Scrivano --- drivers/overlay/overlay.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index b3b551b7e7..605fae8775 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -2029,7 +2029,7 @@ func (d *Driver) DiffGetter(id string) (graphdriver.FileGetCloser, error) { // CleanupStagingDirectory cleanups the staging directory. func (d *Driver) CleanupStagingDirectory(stagingDirectory string) error { - return os.RemoveAll(stagingDirectory) + return os.RemoveAll(filepath.Dir(stagingDirectory)) } func supportsDataOnlyLayersCached(home, runhome string) (bool, error) { @@ -2068,7 +2068,7 @@ func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.App if err != nil && !os.IsExist(err) { return graphdriver.DriverWithDifferOutput{}, err } - applyDir, err = os.MkdirTemp(stagingDir, "") + layerDir, err := os.MkdirTemp(stagingDir, "") if err != nil { return graphdriver.DriverWithDifferOutput{}, err } @@ -2076,7 +2076,8 @@ func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.App if d.options.forceMask != nil { perms = *d.options.forceMask } - if err := os.Chmod(applyDir, perms); err != nil { + applyDir = filepath.Join(layerDir, "dir") + if err := os.Mkdir(applyDir, perms); err != nil { return graphdriver.DriverWithDifferOutput{}, err } } else { @@ -2112,7 +2113,7 @@ func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.App // ApplyDiffFromStagingDirectory applies the changes using the specified staging directory. func (d *Driver) ApplyDiffFromStagingDirectory(id, parent string, diffOutput *graphdriver.DriverWithDifferOutput, options *graphdriver.ApplyDiffWithDifferOpts) error { stagingDirectory := diffOutput.Target - if filepath.Dir(stagingDirectory) != d.getStagingDir(id) { + if filepath.Dir(filepath.Dir(stagingDirectory)) != d.getStagingDir(id) { return fmt.Errorf("%q is not a staging directory", stagingDirectory) } diffPath, err := d.getDiffPath(id) From c06cd1c168049071b1a66fe1a9d287dc4d7d21de Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 14:56:57 +0200 Subject: [PATCH 7/8] lockfile: add functions for non blocking lock extend the public API to allow a non blocking usage. Signed-off-by: Giuseppe Scrivano --- pkg/lockfile/lockfile.go | 54 +++++++++++++++++++++++++++++- pkg/lockfile/lockfile_test.go | 57 ++++++++++++++++++++++++++++++++ pkg/lockfile/lockfile_unix.go | 16 +++++++-- pkg/lockfile/lockfile_windows.go | 13 +++++++- 4 files changed, 136 insertions(+), 4 deletions(-) diff --git a/pkg/lockfile/lockfile.go b/pkg/lockfile/lockfile.go index 19694be3bc..5101475786 100644 --- a/pkg/lockfile/lockfile.go +++ b/pkg/lockfile/lockfile.go @@ -138,6 +138,20 @@ func (l *LockFile) RLock() { l.lock(readLock) } +// TryLock attempts to lock the lockfile as a writer. Panic if the lock is a read-only one. +func (l *LockFile) TryLock() error { + if l.ro { + panic("can't take write lock on read-only lock file") + } else { + return l.tryLock(writeLock) + } +} + +// TryRLock attempts to lock the lockfile as a reader. +func (l *LockFile) TryRLock() error { + return l.tryLock(readLock) +} + // Unlock unlocks the lockfile. func (l *LockFile) Unlock() { l.stateMutex.Lock() @@ -401,9 +415,47 @@ func (l *LockFile) lock(lType lockType) { // Optimization: only use the (expensive) syscall when // the counter is 0. In this case, we're either the first // reader lock or a writer lock. - lockHandle(l.fd, lType) + lockHandle(l.fd, lType, false) + } + l.lockType = lType + l.locked = true + l.counter++ +} + +// lock locks the lockfile via syscall based on the specified type and +// command. +func (l *LockFile) tryLock(lType lockType) error { + var success bool + if lType == readLock { + success = l.rwMutex.TryRLock() + } else { + success = l.rwMutex.TryLock() + } + if !success { + return fmt.Errorf("resource temporarily unavailable") + } + l.stateMutex.Lock() + defer l.stateMutex.Unlock() + if l.counter == 0 { + // If we're the first reference on the lock, we need to open the file again. + fd, err := openLock(l.file, l.ro) + if err != nil { + l.rwMutex.Unlock() + return err + } + l.fd = fd + + // Optimization: only use the (expensive) syscall when + // the counter is 0. In this case, we're either the first + // reader lock or a writer lock. + if err = lockHandle(l.fd, lType, true); err != nil { + closeHandle(fd) + l.rwMutex.Unlock() + return err + } } l.lockType = lType l.locked = true l.counter++ + return nil } diff --git a/pkg/lockfile/lockfile_test.go b/pkg/lockfile/lockfile_test.go index 9512fbf74e..824bb32ae3 100644 --- a/pkg/lockfile/lockfile_test.go +++ b/pkg/lockfile/lockfile_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "sync" "sync/atomic" "testing" @@ -221,6 +222,62 @@ func TestLockfileName(t *testing.T) { assert.NotEmpty(t, l.name, "lockfile name should be recorded correctly") } +func TestTryWriteLockFile(t *testing.T) { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + l, err := getTempLockfile() + require.Nil(t, err, "error getting temporary lock file") + defer os.Remove(l.name) + + err = l.TryLock() + assert.Nil(t, err) + + l.AssertLocked() + + errChan := make(chan error) + go func() { + errChan <- l.TryRLock() + errChan <- l.TryLock() + }() + assert.NotNil(t, <-errChan) + assert.NotNil(t, <-errChan) + + l.Unlock() +} + +func TestTryReadLockFile(t *testing.T) { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + l, err := getTempLockfile() + require.Nil(t, err, "error getting temporary lock file") + defer os.Remove(l.name) + + err = l.TryRLock() + assert.Nil(t, err) + + l.AssertLocked() + + errChan := make(chan error) + go func() { + errChan <- l.TryRLock() + l.Unlock() + + errChan <- l.TryLock() + }() + assert.Nil(t, <-errChan) + assert.NotNil(t, <-errChan) + + l.Unlock() + + go func() { + errChan <- l.TryLock() + l.Unlock() + }() + assert.Nil(t, <-errChan) +} + func TestLockfileRead(t *testing.T) { l, err := getTempLockfile() require.Nil(t, err, "error getting temporary lock file") diff --git a/pkg/lockfile/lockfile_unix.go b/pkg/lockfile/lockfile_unix.go index 38e737e265..0eff003bcd 100644 --- a/pkg/lockfile/lockfile_unix.go +++ b/pkg/lockfile/lockfile_unix.go @@ -74,7 +74,7 @@ func openHandle(path string, mode int) (fileHandle, error) { return fileHandle(fd), err } -func lockHandle(fd fileHandle, lType lockType) { +func lockHandle(fd fileHandle, lType lockType, nonblocking bool) error { fType := unix.F_RDLCK if lType != readLock { fType = unix.F_WRLCK @@ -85,7 +85,15 @@ func lockHandle(fd fileHandle, lType lockType) { Start: 0, Len: 0, } - for unix.FcntlFlock(uintptr(fd), unix.F_SETLKW, &lk) != nil { + cmd := unix.F_SETLKW + if nonblocking { + cmd = unix.F_SETLK + } + for { + err := unix.FcntlFlock(uintptr(fd), cmd, &lk) + if err == nil || nonblocking { + return err + } time.Sleep(10 * time.Millisecond) } } @@ -93,3 +101,7 @@ func lockHandle(fd fileHandle, lType lockType) { func unlockAndCloseHandle(fd fileHandle) { unix.Close(int(fd)) } + +func closeHandle(fd fileHandle) { + unix.Close(int(fd)) +} diff --git a/pkg/lockfile/lockfile_windows.go b/pkg/lockfile/lockfile_windows.go index 304c92b158..6482529b3e 100644 --- a/pkg/lockfile/lockfile_windows.go +++ b/pkg/lockfile/lockfile_windows.go @@ -81,19 +81,30 @@ func openHandle(path string, mode int) (fileHandle, error) { return fileHandle(fd), err } -func lockHandle(fd fileHandle, lType lockType) { +func lockHandle(fd fileHandle, lType lockType, nonblocking bool) error { flags := 0 if lType != readLock { flags = windows.LOCKFILE_EXCLUSIVE_LOCK } + if nonblocking { + flags |= windows.LOCKFILE_FAIL_IMMEDIATELY + } ol := new(windows.Overlapped) if err := windows.LockFileEx(windows.Handle(fd), uint32(flags), reserved, allBytes, allBytes, ol); err != nil { + if nonblocking { + return err + } panic(err) } + return nil } func unlockAndCloseHandle(fd fileHandle) { ol := new(windows.Overlapped) windows.UnlockFileEx(windows.Handle(fd), reserved, allBytes, allBytes, ol) + closeHandle(fd) +} + +func closeHandle(fd fileHandle) { windows.Close(windows.Handle(fd)) } From f1352dfe13ecccf2abf09ec7b3ee4301f98dbf79 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 30 Apr 2024 15:15:45 +0200 Subject: [PATCH 8/8] overlay: lock staging directories lock any staging directory while it is being used so that another process cannot delete it. Now the Cleanup() function deletes only the staging directories that are not locked by any other user. Closes: https://github.com/containers/storage/issues/1915 Signed-off-by: Giuseppe Scrivano --- drivers/overlay/overlay.go | 76 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 605fae8775..91f457ce20 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -28,6 +28,7 @@ import ( "github.com/containers/storage/pkg/fsutils" "github.com/containers/storage/pkg/idmap" "github.com/containers/storage/pkg/idtools" + "github.com/containers/storage/pkg/lockfile" "github.com/containers/storage/pkg/mount" "github.com/containers/storage/pkg/parsers" "github.com/containers/storage/pkg/system" @@ -83,6 +84,8 @@ const ( lowerFile = "lower" maxDepth = 500 + stagingLockFile = "staging.lock" + tocArtifact = "toc" fsVerityDigestsArtifact = "fs-verity-digests" @@ -127,6 +130,8 @@ type Driver struct { usingMetacopy bool usingComposefs bool + stagingDirsLocks map[string]*lockfile.LockFile + supportsIDMappedMounts *bool } @@ -460,6 +465,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) supportsVolatile: supportsVolatile, usingComposefs: opts.useComposefs, options: *opts, + stagingDirsLocks: make(map[string]*lockfile.LockFile), } d.naiveDiff = graphdriver.NewNaiveDiffDriver(d, graphdriver.NewNaiveLayerIDMapUpdater(d)) @@ -880,10 +886,44 @@ func (d *Driver) Metadata(id string) (map[string]string, error) { // the storage is being shutdown. The only state created by the driver // is the bind mount on the home directory. func (d *Driver) Cleanup() error { - _ = os.RemoveAll(filepath.Join(d.home, stagingDir)) + anyPresent := d.pruneStagingDirectories() + if anyPresent { + return nil + } return mount.Unmount(d.home) } +// pruneStagingDirectories cleans up any staging directory that was leaked. +// It returns whether any staging directory is still present. +func (d *Driver) pruneStagingDirectories() bool { + for _, lock := range d.stagingDirsLocks { + lock.Unlock() + } + d.stagingDirsLocks = make(map[string]*lockfile.LockFile) + + anyPresent := false + + homeStagingDir := filepath.Join(d.home, stagingDir) + dirs, err := os.ReadDir(homeStagingDir) + if err == nil { + for _, dir := range dirs { + stagingDirToRemove := filepath.Join(homeStagingDir, dir.Name()) + lock, err := lockfile.GetLockFile(filepath.Join(stagingDirToRemove, stagingLockFile)) + if err != nil { + anyPresent = true + continue + } + if err := lock.TryLock(); err != nil { + anyPresent = true + continue + } + _ = os.RemoveAll(stagingDirToRemove) + lock.Unlock() + } + } + return anyPresent +} + // LookupAdditionalLayer looks up additional layer store by the specified // digest and ref and returns an object representing that layer. // This API is experimental and can be changed without bumping the major version number. @@ -2029,7 +2069,14 @@ func (d *Driver) DiffGetter(id string) (graphdriver.FileGetCloser, error) { // CleanupStagingDirectory cleanups the staging directory. func (d *Driver) CleanupStagingDirectory(stagingDirectory string) error { - return os.RemoveAll(filepath.Dir(stagingDirectory)) + parentStagingDir := filepath.Dir(stagingDirectory) + + if lock, ok := d.stagingDirsLocks[parentStagingDir]; ok { + delete(d.stagingDirsLocks, parentStagingDir) + lock.Unlock() + } + + return os.RemoveAll(parentStagingDir) } func supportsDataOnlyLayersCached(home, runhome string) (bool, error) { @@ -2080,6 +2127,19 @@ func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.App if err := os.Mkdir(applyDir, perms); err != nil { return graphdriver.DriverWithDifferOutput{}, err } + + lock, err := lockfile.GetLockFile(filepath.Join(layerDir, stagingLockFile)) + if err != nil { + return graphdriver.DriverWithDifferOutput{}, err + } + defer func() { + if errRet != nil { + delete(d.stagingDirsLocks, layerDir) + lock.Unlock() + } + }() + d.stagingDirsLocks[layerDir] = lock + lock.Lock() } else { var err error applyDir, err = d.getDiffPath(id) @@ -2113,9 +2173,19 @@ func (d *Driver) ApplyDiffWithDiffer(id, parent string, options *graphdriver.App // ApplyDiffFromStagingDirectory applies the changes using the specified staging directory. func (d *Driver) ApplyDiffFromStagingDirectory(id, parent string, diffOutput *graphdriver.DriverWithDifferOutput, options *graphdriver.ApplyDiffWithDifferOpts) error { stagingDirectory := diffOutput.Target - if filepath.Dir(filepath.Dir(stagingDirectory)) != d.getStagingDir(id) { + parentStagingDir := filepath.Dir(stagingDirectory) + + defer func() { + if lock, ok := d.stagingDirsLocks[parentStagingDir]; ok { + delete(d.stagingDirsLocks, parentStagingDir) + lock.Unlock() + } + }() + + if filepath.Dir(parentStagingDir) != d.getStagingDir(id) { return fmt.Errorf("%q is not a staging directory", stagingDirectory) } + diffPath, err := d.getDiffPath(id) if err != nil { return err