Skip to content

Commit

Permalink
storage: ensure no filesystem use after close in test builds
Browse files Browse the repository at this point in the history
In test builds, panic if any of the VFS filesystem facilities are used after
the Engine is closed.

Epic: none
Informs #106119.
Release note: None
  • Loading branch information
jbowens committed Sep 14, 2023
1 parent ef71b2f commit 9da340c
Show file tree
Hide file tree
Showing 2 changed files with 185 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/storage/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,7 @@ func TestEngineFS(t *testing.T) {
"8c: f.close",
"8d: f = open /bar",
"8e: f.read 3 == ghe",
"8f: f.close",
"9a: create-dir /dir1",
"9b: create /dir1/bar",
"9c: list-dir /dir1 == bar",
Expand Down
184 changes: 184 additions & 0 deletions pkg/storage/pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,18 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) {
onClose: cfg.onClose,
replayer: replay.NewWorkloadCollector(cfg.StorageConfig.Dir),
}
// In test builds, add a layer of VFS middleware that ensures users of an
// Engine don't try to use the filesystem after the Engine has been closed.
// Usage after close causes goroutine leaks.
if buildutil.CrdbTestBuild {
testFS := &noUseAfterClose{closed: &p.closed, fs: p.FS}
p.FS = testFS
p.onClose = append(p.onClose, func(p *Pebble) {
if openFiles := testFS.openFiles.Load(); openFiles > 0 {
panic(errors.AssertionFailedf("during Engine.Close there remain %d files open by client code; all files must be closed before closing the Engine", openFiles))
}
})
}

// MaxConcurrentCompactions can be set by multiple sources, but all the
// sources will eventually call NewPebble. So, we override
Expand Down Expand Up @@ -2908,3 +2920,175 @@ var _ error = &ExceedMaxSizeError{}
func (e *ExceedMaxSizeError) Error() string {
return fmt.Sprintf("export size (%d bytes) exceeds max size (%d bytes)", e.reached, e.maxSize)
}

// noUseAfterClose wraps a vfs.FS and ensures the filesystem is not used after
// an Engine is closed. It's used only in test builds. It ensures that all files
// are closed before the Engine is closed and that the vfs.FS is not used after
// the Engine is closed.
type noUseAfterClose struct {
fs vfs.FS
openFiles atomic.Int32
closed *bool
}

func (fs *noUseAfterClose) ensureStillOpen() {
if *fs.closed {
panic(errors.AssertionFailedf("engine is closed; the Engine-provided filesystem cannot be used after Close"))
}
}

type noUseAfterCloseFile struct {
file vfs.File
fs *noUseAfterClose
}

func (f *noUseAfterCloseFile) Close() error {
f.fs.ensureStillOpen()
f.fs.openFiles.Add(-1)
return f.file.Close()
}
func (f *noUseAfterCloseFile) Read(p []byte) (n int, err error) {
f.fs.ensureStillOpen()
return f.file.Read(p)
}
func (f *noUseAfterCloseFile) ReadAt(p []byte, off int64) (n int, err error) {
f.fs.ensureStillOpen()
return f.file.ReadAt(p, off)
}
func (f *noUseAfterCloseFile) Write(p []byte) (n int, err error) {
f.fs.ensureStillOpen()
return f.file.Write(p)
}
func (f *noUseAfterCloseFile) WriteAt(p []byte, off int64) (n int, err error) {
f.fs.ensureStillOpen()
return f.file.WriteAt(p, off)
}
func (f *noUseAfterCloseFile) Preallocate(offset, length int64) error {
f.fs.ensureStillOpen()
return f.file.Preallocate(offset, length)
}
func (f *noUseAfterCloseFile) Stat() (os.FileInfo, error) {
f.fs.ensureStillOpen()
return f.file.Stat()
}
func (f *noUseAfterCloseFile) Sync() error {
f.fs.ensureStillOpen()
return f.file.Sync()
}
func (f *noUseAfterCloseFile) SyncTo(length int64) (fullSync bool, err error) {
f.fs.ensureStillOpen()
return f.file.SyncTo(length)
}
func (f *noUseAfterCloseFile) SyncData() error {
f.fs.ensureStillOpen()
return f.file.SyncData()
}
func (f *noUseAfterCloseFile) Prefetch(offset int64, length int64) error {
f.fs.ensureStillOpen()
return f.file.Prefetch(offset, length)
}
func (f *noUseAfterCloseFile) Fd() uintptr { return f.file.Fd() }

var _ vfs.FS = &noUseAfterClose{}

func (fs *noUseAfterClose) Create(name string) (vfs.File, error) {
fs.ensureStillOpen()
return fs.fs.Create(name)
}

func (fs *noUseAfterClose) Link(oldname, newname string) error {
fs.ensureStillOpen()
return fs.fs.Link(oldname, newname)
}

func (fs *noUseAfterClose) Open(name string, opts ...vfs.OpenOption) (vfs.File, error) {
fs.ensureStillOpen()
f, err := fs.fs.Open(name, opts...)
if err != nil {
return nil, err
}
fs.openFiles.Add(1)
return &noUseAfterCloseFile{fs: fs, file: f}, nil
}

func (fs *noUseAfterClose) OpenReadWrite(name string, opts ...vfs.OpenOption) (vfs.File, error) {
fs.ensureStillOpen()
f, err := fs.fs.OpenReadWrite(name, opts...)
if err != nil {
return nil, err
}
fs.openFiles.Add(1)
return &noUseAfterCloseFile{fs: fs, file: f}, nil
}

func (fs *noUseAfterClose) OpenDir(name string) (vfs.File, error) {
fs.ensureStillOpen()
f, err := fs.fs.OpenDir(name)
if err != nil {
return nil, err
}
fs.openFiles.Add(1)
return &noUseAfterCloseFile{fs: fs, file: f}, nil
}

func (fs *noUseAfterClose) Remove(name string) error {
fs.ensureStillOpen()
return fs.fs.Remove(name)
}

func (fs *noUseAfterClose) RemoveAll(name string) error {
fs.ensureStillOpen()
return fs.fs.RemoveAll(name)
}

func (fs *noUseAfterClose) Rename(oldname, newname string) error {
fs.ensureStillOpen()
return fs.fs.Rename(oldname, newname)
}

func (fs *noUseAfterClose) ReuseForWrite(oldname, newname string) (vfs.File, error) {
fs.ensureStillOpen()
f, err := fs.fs.ReuseForWrite(oldname, newname)
if err != nil {
return nil, err
}
fs.openFiles.Add(1)
return &noUseAfterCloseFile{fs: fs, file: f}, nil
}

func (fs *noUseAfterClose) MkdirAll(dir string, perm os.FileMode) error {
fs.ensureStillOpen()
return fs.fs.MkdirAll(dir, perm)
}

func (fs *noUseAfterClose) Lock(name string) (io.Closer, error) {
fs.ensureStillOpen()
return fs.fs.Lock(name)
}

func (fs *noUseAfterClose) List(dir string) ([]string, error) {
fs.ensureStillOpen()
return fs.fs.List(dir)
}

func (fs *noUseAfterClose) Stat(name string) (os.FileInfo, error) {
fs.ensureStillOpen()
return fs.fs.Stat(name)
}

func (fs *noUseAfterClose) PathBase(path string) string {
return fs.fs.PathBase(path)
}

func (fs *noUseAfterClose) PathJoin(elem ...string) string {
return fs.fs.PathJoin(elem...)
}

func (fs *noUseAfterClose) PathDir(path string) string {
return fs.fs.PathDir(path)
}

func (fs *noUseAfterClose) GetDiskUsage(path string) (vfs.DiskUsage, error) {
fs.ensureStillOpen()
return fs.fs.GetDiskUsage(path)
}

0 comments on commit 9da340c

Please sign in to comment.