diff options
Diffstat (limited to 'pkg/sentry/fsimpl')
21 files changed, 266 insertions, 364 deletions
diff --git a/pkg/sentry/fsimpl/devpts/devpts_state_autogen.go b/pkg/sentry/fsimpl/devpts/devpts_state_autogen.go index dd9e03e42..1a294b64c 100644 --- a/pkg/sentry/fsimpl/devpts/devpts_state_autogen.go +++ b/pkg/sentry/fsimpl/devpts/devpts_state_autogen.go @@ -419,10 +419,9 @@ func (r *rootInodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *rootInodeRefs) afterLoad() {} - func (r *rootInodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (tm *Terminal) StateTypeName() string { diff --git a/pkg/sentry/fsimpl/devpts/root_inode_refs.go b/pkg/sentry/fsimpl/devpts/root_inode_refs.go index 051801202..577b92f36 100644 --- a/pkg/sentry/fsimpl/devpts/root_inode_refs.go +++ b/pkg/sentry/fsimpl/devpts/root_inode_refs.go @@ -2,11 +2,9 @@ package devpts import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var rootInodeownerType *rootInode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type rootInodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type rootInodeRefs struct { refCount int64 } -func (r *rootInodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, rootInodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *rootInodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", rootInodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *rootInodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*rootInodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *rootInodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", rootInodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *rootInodeRefs) ReadRefs() int64 { //go:nosplit func (r *rootInodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, rootInodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, rootInodeownerType)) } } @@ -110,9 +95,18 @@ func (r *rootInodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, rootInodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", rootInodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *rootInodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/fuse/fuse_state_autogen.go b/pkg/sentry/fsimpl/fuse/fuse_state_autogen.go index 4c8bc4410..f59e82755 100644 --- a/pkg/sentry/fsimpl/fuse/fuse_state_autogen.go +++ b/pkg/sentry/fsimpl/fuse/fuse_state_autogen.go @@ -346,10 +346,9 @@ func (r *inodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *inodeRefs) afterLoad() {} - func (r *inodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (l *requestList) StateTypeName() string { diff --git a/pkg/sentry/fsimpl/fuse/inode_refs.go b/pkg/sentry/fsimpl/fuse/inode_refs.go index 6b9456e1d..970f90a91 100644 --- a/pkg/sentry/fsimpl/fuse/inode_refs.go +++ b/pkg/sentry/fsimpl/fuse/inode_refs.go @@ -2,11 +2,9 @@ package fuse import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var inodeownerType *inode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type inodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type inodeRefs struct { refCount int64 } -func (r *inodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, inodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *inodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", inodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *inodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*inodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *inodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", inodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *inodeRefs) ReadRefs() int64 { //go:nosplit func (r *inodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, inodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, inodeownerType)) } } @@ -110,9 +95,18 @@ func (r *inodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, inodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", inodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *inodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/host/connected_endpoint_refs.go b/pkg/sentry/fsimpl/host/connected_endpoint_refs.go index babb3f664..6d2f22c8d 100644 --- a/pkg/sentry/fsimpl/host/connected_endpoint_refs.go +++ b/pkg/sentry/fsimpl/host/connected_endpoint_refs.go @@ -2,11 +2,9 @@ package host import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var ConnectedEndpointownerType *ConnectedEndpoint // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type ConnectedEndpointRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type ConnectedEndpointRefs struct { refCount int64 } -func (r *ConnectedEndpointRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, ConnectedEndpointownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *ConnectedEndpointRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", ConnectedEndpointownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *ConnectedEndpointRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*ConnectedEndpointRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *ConnectedEndpointRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", ConnectedEndpointownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *ConnectedEndpointRefs) ReadRefs() int64 { //go:nosplit func (r *ConnectedEndpointRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, ConnectedEndpointownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, ConnectedEndpointownerType)) } } @@ -110,9 +95,18 @@ func (r *ConnectedEndpointRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, ConnectedEndpointownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", ConnectedEndpointownerType)) + } if destroy != nil { destroy() } } } + +func (r *ConnectedEndpointRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/host/host_state_autogen.go b/pkg/sentry/fsimpl/host/host_state_autogen.go index 5aaee37c3..3507e1aa1 100644 --- a/pkg/sentry/fsimpl/host/host_state_autogen.go +++ b/pkg/sentry/fsimpl/host/host_state_autogen.go @@ -23,10 +23,9 @@ func (r *ConnectedEndpointRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *ConnectedEndpointRefs) afterLoad() {} - func (r *ConnectedEndpointRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (f *filesystemType) StateTypeName() string { @@ -191,10 +190,9 @@ func (r *inodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *inodeRefs) afterLoad() {} - func (r *inodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (i *inodePlatformFile) StateTypeName() string { diff --git a/pkg/sentry/fsimpl/host/inode_refs.go b/pkg/sentry/fsimpl/host/inode_refs.go index 17f90ce4a..3504cc603 100644 --- a/pkg/sentry/fsimpl/host/inode_refs.go +++ b/pkg/sentry/fsimpl/host/inode_refs.go @@ -2,11 +2,9 @@ package host import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var inodeownerType *inode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type inodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type inodeRefs struct { refCount int64 } -func (r *inodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, inodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *inodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", inodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *inodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*inodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *inodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", inodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *inodeRefs) ReadRefs() int64 { //go:nosplit func (r *inodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, inodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, inodeownerType)) } } @@ -110,9 +95,18 @@ func (r *inodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, inodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", inodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *inodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/kernfs/dentry_refs.go b/pkg/sentry/fsimpl/kernfs/dentry_refs.go index 79863b3bc..c2304939b 100644 --- a/pkg/sentry/fsimpl/kernfs/dentry_refs.go +++ b/pkg/sentry/fsimpl/kernfs/dentry_refs.go @@ -2,11 +2,9 @@ package kernfs import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var DentryownerType *Dentry // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type DentryRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type DentryRefs struct { refCount int64 } -func (r *DentryRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, DentryownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *DentryRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", DentryownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *DentryRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*DentryRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *DentryRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", DentryownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *DentryRefs) ReadRefs() int64 { //go:nosplit func (r *DentryRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, DentryownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, DentryownerType)) } } @@ -110,9 +95,18 @@ func (r *DentryRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, DentryownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", DentryownerType)) + } if destroy != nil { destroy() } } } + +func (r *DentryRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/kernfs/kernfs_state_autogen.go b/pkg/sentry/fsimpl/kernfs/kernfs_state_autogen.go index f87782ee1..5121f8225 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs_state_autogen.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs_state_autogen.go @@ -23,10 +23,9 @@ func (r *DentryRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *DentryRefs) afterLoad() {} - func (r *DentryRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (f *DynamicBytesFile) StateTypeName() string { @@ -677,10 +676,9 @@ func (r *StaticDirectoryRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *StaticDirectoryRefs) afterLoad() {} - func (r *StaticDirectoryRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (s *StaticSymlink) StateTypeName() string { @@ -776,10 +774,9 @@ func (r *syntheticDirectoryRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *syntheticDirectoryRefs) afterLoad() {} - func (r *syntheticDirectoryRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func init() { diff --git a/pkg/sentry/fsimpl/kernfs/static_directory_refs.go b/pkg/sentry/fsimpl/kernfs/static_directory_refs.go index 478b04bdd..9472a96b9 100644 --- a/pkg/sentry/fsimpl/kernfs/static_directory_refs.go +++ b/pkg/sentry/fsimpl/kernfs/static_directory_refs.go @@ -2,11 +2,9 @@ package kernfs import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var StaticDirectoryownerType *StaticDirectory // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type StaticDirectoryRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type StaticDirectoryRefs struct { refCount int64 } -func (r *StaticDirectoryRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, StaticDirectoryownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *StaticDirectoryRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", StaticDirectoryownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *StaticDirectoryRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*StaticDirectoryRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *StaticDirectoryRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", StaticDirectoryownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *StaticDirectoryRefs) ReadRefs() int64 { //go:nosplit func (r *StaticDirectoryRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, StaticDirectoryownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, StaticDirectoryownerType)) } } @@ -110,9 +95,18 @@ func (r *StaticDirectoryRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, StaticDirectoryownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", StaticDirectoryownerType)) + } if destroy != nil { destroy() } } } + +func (r *StaticDirectoryRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/kernfs/synthetic_directory_refs.go b/pkg/sentry/fsimpl/kernfs/synthetic_directory_refs.go index 28d556b42..7c4fde369 100644 --- a/pkg/sentry/fsimpl/kernfs/synthetic_directory_refs.go +++ b/pkg/sentry/fsimpl/kernfs/synthetic_directory_refs.go @@ -2,11 +2,9 @@ package kernfs import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var syntheticDirectoryownerType *syntheticDirectory // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type syntheticDirectoryRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type syntheticDirectoryRefs struct { refCount int64 } -func (r *syntheticDirectoryRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, syntheticDirectoryownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *syntheticDirectoryRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", syntheticDirectoryownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *syntheticDirectoryRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*syntheticDirectoryRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *syntheticDirectoryRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", syntheticDirectoryownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *syntheticDirectoryRefs) ReadRefs() int64 { //go:nosplit func (r *syntheticDirectoryRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, syntheticDirectoryownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, syntheticDirectoryownerType)) } } @@ -110,9 +95,18 @@ func (r *syntheticDirectoryRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, syntheticDirectoryownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", syntheticDirectoryownerType)) + } if destroy != nil { destroy() } } } + +func (r *syntheticDirectoryRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go b/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go index 9431c1506..2f0dd126e 100644 --- a/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go +++ b/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go @@ -2,11 +2,9 @@ package proc import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var fdDirInodeownerType *fdDirInode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type fdDirInodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type fdDirInodeRefs struct { refCount int64 } -func (r *fdDirInodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, fdDirInodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *fdDirInodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", fdDirInodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *fdDirInodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*fdDirInodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *fdDirInodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", fdDirInodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *fdDirInodeRefs) ReadRefs() int64 { //go:nosplit func (r *fdDirInodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, fdDirInodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, fdDirInodeownerType)) } } @@ -110,9 +95,18 @@ func (r *fdDirInodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, fdDirInodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", fdDirInodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *fdDirInodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go b/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go index 872b20eb0..2065c97b4 100644 --- a/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go +++ b/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go @@ -2,11 +2,9 @@ package proc import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var fdInfoDirInodeownerType *fdInfoDirInode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type fdInfoDirInodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type fdInfoDirInodeRefs struct { refCount int64 } -func (r *fdInfoDirInodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, fdInfoDirInodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *fdInfoDirInodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", fdInfoDirInodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *fdInfoDirInodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*fdInfoDirInodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *fdInfoDirInodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", fdInfoDirInodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *fdInfoDirInodeRefs) ReadRefs() int64 { //go:nosplit func (r *fdInfoDirInodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, fdInfoDirInodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, fdInfoDirInodeownerType)) } } @@ -110,9 +95,18 @@ func (r *fdInfoDirInodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, fdInfoDirInodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", fdInfoDirInodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *fdInfoDirInodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/proc/proc_state_autogen.go b/pkg/sentry/fsimpl/proc/proc_state_autogen.go index e17a2a13c..1b5e98012 100644 --- a/pkg/sentry/fsimpl/proc/proc_state_autogen.go +++ b/pkg/sentry/fsimpl/proc/proc_state_autogen.go @@ -23,10 +23,9 @@ func (r *fdDirInodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *fdDirInodeRefs) afterLoad() {} - func (r *fdDirInodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (r *fdInfoDirInodeRefs) StateTypeName() string { @@ -46,10 +45,9 @@ func (r *fdInfoDirInodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *fdInfoDirInodeRefs) afterLoad() {} - func (r *fdInfoDirInodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (ft *FilesystemType) StateTypeName() string { @@ -267,10 +265,9 @@ func (r *subtasksInodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *subtasksInodeRefs) afterLoad() {} - func (r *subtasksInodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (i *taskInode) StateTypeName() string { @@ -1101,10 +1098,9 @@ func (r *taskInodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *taskInodeRefs) afterLoad() {} - func (r *taskInodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (n *ifinet6) StateTypeName() string { @@ -1747,10 +1743,9 @@ func (r *tasksInodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *tasksInodeRefs) afterLoad() {} - func (r *tasksInodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (t *tcpMemDir) StateTypeName() string { diff --git a/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go b/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go index c6d9b3522..c4c0baf31 100644 --- a/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go +++ b/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go @@ -2,11 +2,9 @@ package proc import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var subtasksInodeownerType *subtasksInode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type subtasksInodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type subtasksInodeRefs struct { refCount int64 } -func (r *subtasksInodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, subtasksInodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *subtasksInodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", subtasksInodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *subtasksInodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*subtasksInodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *subtasksInodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", subtasksInodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *subtasksInodeRefs) ReadRefs() int64 { //go:nosplit func (r *subtasksInodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, subtasksInodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, subtasksInodeownerType)) } } @@ -110,9 +95,18 @@ func (r *subtasksInodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, subtasksInodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", subtasksInodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *subtasksInodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/proc/task_inode_refs.go b/pkg/sentry/fsimpl/proc/task_inode_refs.go index 714488450..67638f6ae 100644 --- a/pkg/sentry/fsimpl/proc/task_inode_refs.go +++ b/pkg/sentry/fsimpl/proc/task_inode_refs.go @@ -2,11 +2,9 @@ package proc import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var taskInodeownerType *taskInode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type taskInodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type taskInodeRefs struct { refCount int64 } -func (r *taskInodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, taskInodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *taskInodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", taskInodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *taskInodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*taskInodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *taskInodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", taskInodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *taskInodeRefs) ReadRefs() int64 { //go:nosplit func (r *taskInodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, taskInodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, taskInodeownerType)) } } @@ -110,9 +95,18 @@ func (r *taskInodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, taskInodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", taskInodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *taskInodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/proc/tasks_inode_refs.go b/pkg/sentry/fsimpl/proc/tasks_inode_refs.go index 22d9cc488..b882335d7 100644 --- a/pkg/sentry/fsimpl/proc/tasks_inode_refs.go +++ b/pkg/sentry/fsimpl/proc/tasks_inode_refs.go @@ -2,11 +2,9 @@ package proc import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var tasksInodeownerType *tasksInode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type tasksInodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type tasksInodeRefs struct { refCount int64 } -func (r *tasksInodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, tasksInodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *tasksInodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", tasksInodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *tasksInodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*tasksInodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *tasksInodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", tasksInodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *tasksInodeRefs) ReadRefs() int64 { //go:nosplit func (r *tasksInodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, tasksInodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, tasksInodeownerType)) } } @@ -110,9 +95,18 @@ func (r *tasksInodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, tasksInodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", tasksInodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *tasksInodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/sys/dir_refs.go b/pkg/sentry/fsimpl/sys/dir_refs.go index 89609b198..371ad3a8c 100644 --- a/pkg/sentry/fsimpl/sys/dir_refs.go +++ b/pkg/sentry/fsimpl/sys/dir_refs.go @@ -2,11 +2,9 @@ package sys import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var dirownerType *dir // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type dirRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type dirRefs struct { refCount int64 } -func (r *dirRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, dirownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *dirRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", dirownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *dirRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*dirRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *dirRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", dirownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *dirRefs) ReadRefs() int64 { //go:nosplit func (r *dirRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, dirownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, dirownerType)) } } @@ -110,9 +95,18 @@ func (r *dirRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, dirownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", dirownerType)) + } if destroy != nil { destroy() } } } + +func (r *dirRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/sys/sys_state_autogen.go b/pkg/sentry/fsimpl/sys/sys_state_autogen.go index 64c9c9d1f..13cbe9a90 100644 --- a/pkg/sentry/fsimpl/sys/sys_state_autogen.go +++ b/pkg/sentry/fsimpl/sys/sys_state_autogen.go @@ -23,10 +23,9 @@ func (r *dirRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *dirRefs) afterLoad() {} - func (r *dirRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (i *kcovInode) StateTypeName() string { diff --git a/pkg/sentry/fsimpl/tmpfs/inode_refs.go b/pkg/sentry/fsimpl/tmpfs/inode_refs.go index dbf0b2766..55b1d39fe 100644 --- a/pkg/sentry/fsimpl/tmpfs/inode_refs.go +++ b/pkg/sentry/fsimpl/tmpfs/inode_refs.go @@ -2,11 +2,9 @@ package tmpfs import ( "fmt" - "runtime" "sync/atomic" - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" ) // ownerType is used to customize logging. Note that we use a pointer to T so @@ -19,11 +17,6 @@ var inodeownerType *inode // Note that the number of references is actually refCount + 1 so that a default // zero-value Refs object contains one reference. // -// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in -// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount. -// This will allow us to add stack trace information to the leak messages -// without growing the size of Refs. -// // +stateify savable type inodeRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type inodeRefs struct { refCount int64 } -func (r *inodeRefs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, inodeownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *inodeRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", inodeownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *inodeRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*inodeRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *inodeRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", inodeownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *inodeRefs) ReadRefs() int64 { //go:nosplit func (r *inodeRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, inodeownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, inodeownerType)) } } @@ -110,9 +95,18 @@ func (r *inodeRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, inodeownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", inodeownerType)) + } if destroy != nil { destroy() } } } + +func (r *inodeRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs_state_autogen.go b/pkg/sentry/fsimpl/tmpfs/tmpfs_state_autogen.go index 1b6127f5d..21681ba5c 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs_state_autogen.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs_state_autogen.go @@ -174,10 +174,9 @@ func (r *inodeRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *inodeRefs) afterLoad() {} - func (r *inodeRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (n *namedPipe) StateTypeName() string { |