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 cockroachdb#106119.
Release note: None
  • Loading branch information
jbowens authored and yuzefovich committed Sep 8, 2023
1 parent 06691b7 commit 8259f91
Show file tree
Hide file tree
Showing 2 changed files with 181 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
180 changes: 180 additions & 0 deletions pkg/storage/pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,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 @@ -2763,3 +2775,171 @@ var _ error = &ExceedMaxSizeError{}
func (e *ExceedMaxSizeError) Error() string {
return fmt.Sprintf("export size (%d bytes) exceeds max size (%d bytes)", e.reached, e.maxSize)
}

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 8259f91

Please sign in to comment.