diff --git a/pkg/sentry/fs/fs.go b/pkg/sentry/fs/fs.go index b5c72990e..0ba4b7269 100644 --- a/pkg/sentry/fs/fs.go +++ b/pkg/sentry/fs/fs.go @@ -44,7 +44,7 @@ // DirentCache.mu // Locks in InodeOperations implementations or overlayEntry // Inode.Watches.mu (see `Inotify` for other lock ordering) -// MountSource.mu +// MountSource.mu // // If multiple Dirent or MountSource locks must be taken, locks in the parent must be // taken before locks in their children. @@ -60,10 +60,11 @@ import ( ) var ( - // work is a sync.WaitGroup that can be used to queue asynchronous - // operations via Do. Callers can use Barrier to ensure no operations - // are outstanding. - work sync.WaitGroup + // workMu is used to synchronize pending asynchronous work. Async work + // runs with the lock held for reading. AsyncBarrier will take the lock + // for writing, thus ensuring that all Async work completes before + // AsyncBarrier returns. + workMu sync.RWMutex // asyncError is used to store up to one asynchronous execution error. asyncError = make(chan error, 1) @@ -71,14 +72,17 @@ var ( // AsyncBarrier waits for all outstanding asynchronous work to complete. func AsyncBarrier() { - work.Wait() + workMu.Lock() + workMu.Unlock() } // Async executes a function asynchronously. +// +// Async must not be called recursively. func Async(f func()) { - work.Add(1) - go func() { // S/R-SAFE: Barrier must be called. - defer work.Done() // Ensure Done in case of panic. + workMu.RLock() + go func() { // S/R-SAFE: AsyncBarrier must be called. + defer workMu.RUnlock() // Ensure RUnlock in case of panic. f() }() } @@ -89,7 +93,7 @@ func Async(f func()) { func AsyncErrorBarrier() error { wait := make(chan struct{}, 1) go func() { // S/R-SAFE: Does not touch persistent state. - work.Wait() + AsyncBarrier() wait <- struct{}{} }() select { diff --git a/pkg/sentry/fs/gofer/inode.go b/pkg/sentry/fs/gofer/inode.go index 5811b8b12..7c6e5b025 100644 --- a/pkg/sentry/fs/gofer/inode.go +++ b/pkg/sentry/fs/gofer/inode.go @@ -333,8 +333,14 @@ func (i *inodeOperations) session() *session { // Release implements fs.InodeOperations.Release. func (i *inodeOperations) Release(ctx context.Context) { - i.fileState.Release(ctx) i.cachingInodeOps.Release() + + // Releasing the fileState may make RPCs to the gofer. There is + // no need to wait for those to return, so we can do this + // asynchronously. + fs.Async(func() { + i.fileState.Release(ctx) + }) } // Mappable implements fs.InodeOperations.Mappable. diff --git a/pkg/sentry/fs/inode.go b/pkg/sentry/fs/inode.go index 38b140bd2..fa3beb111 100644 --- a/pkg/sentry/fs/inode.go +++ b/pkg/sentry/fs/inode.go @@ -110,20 +110,13 @@ func (i *Inode) destroy() { // wouldn't be in the destructor. i.Watches.targetDestroyed() - // Overlay resources should be released synchronously, since they may - // trigger more Inode.destroy calls which must themselves be handled - // synchronously, like the WriteOut call above. if i.overlay != nil { i.overlay.release() - i.MountSource.DecRef() - return + } else { + i.InodeOperations.Release(ctx) } - // Regular (non-overlay) resources may be released asynchronously. - Async(func() { - i.InodeOperations.Release(ctx) - i.MountSource.DecRef() - }) + i.MountSource.DecRef() } // Mappable calls i.InodeOperations.Mappable.