diff options
55 files changed, 680 insertions, 770 deletions
diff --git a/pkg/refs_vfs2/refs.go b/pkg/refsvfs2/refs.go index 99a074e96..ef8beb659 100644 --- a/pkg/refs_vfs2/refs.go +++ b/pkg/refsvfs2/refs.go @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package refs_vfs2 defines an interface for a reference-counted object. -package refs_vfs2 +// Package refsvfs2 defines an interface for a reference-counted object. +package refsvfs2 import ( "gvisor.dev/gvisor/pkg/context" diff --git a/pkg/refsvfs2/refs_map.go b/pkg/refsvfs2/refs_map.go new file mode 100644 index 000000000..be75b0cc2 --- /dev/null +++ b/pkg/refsvfs2/refs_map.go @@ -0,0 +1,97 @@ +// Copyright 2020 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package refsvfs2 + +import ( + "fmt" + "strings" + + "gvisor.dev/gvisor/pkg/log" + refs_vfs1 "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/sync" +) + +// TODO(gvisor.dev/issue/1193): re-enable once kernfs refs are fixed. +var ignored []string = []string{"kernfs.", "proc.", "sys.", "devpts.", "fuse."} + +var ( + // liveObjects is a global map of reference-counted objects. Objects are + // inserted when leak check is enabled, and they are removed when they are + // destroyed. It is protected by liveObjectsMu. + liveObjects map[CheckedObject]struct{} + liveObjectsMu sync.Mutex +) + +// CheckedObject represents a reference-counted object with an informative +// leak detection message. +type CheckedObject interface { + // LeakMessage supplies a warning to be printed upon leak detection. + LeakMessage() string +} + +func init() { + liveObjects = make(map[CheckedObject]struct{}) +} + +// LeakCheckEnabled returns whether leak checking is enabled. The following +// functions should only be called if it returns true. +func LeakCheckEnabled() bool { + return refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking +} + +// Register adds obj to the live object map. +func Register(obj CheckedObject, typ string) { + for _, str := range ignored { + if strings.Contains(typ, str) { + return + } + } + liveObjectsMu.Lock() + if _, ok := liveObjects[obj]; ok { + panic(fmt.Sprintf("Unexpected entry in leak checking map: reference %p already added", obj)) + } + liveObjects[obj] = struct{}{} + liveObjectsMu.Unlock() +} + +// Unregister removes obj from the live object map. +func Unregister(obj CheckedObject, typ string) { + liveObjectsMu.Lock() + defer liveObjectsMu.Unlock() + if _, ok := liveObjects[obj]; !ok { + for _, str := range ignored { + if strings.Contains(typ, str) { + return + } + } + panic(fmt.Sprintf("Expected to find entry in leak checking map for reference %p", obj)) + } + delete(liveObjects, obj) +} + +// DoLeakCheck iterates through the live object map and logs a message for each +// object. It is called once no reference-counted objects should be reachable +// anymore, at which point anything left in the map is considered a leak. +func DoLeakCheck() { + liveObjectsMu.Lock() + defer liveObjectsMu.Unlock() + leaked := len(liveObjects) + if leaked > 0 { + log.Warningf("Leak checking detected %d leaked objects:", leaked) + for obj := range liveObjects { + log.Warningf(obj.LeakMessage()) + } + } +} diff --git a/pkg/refs_vfs2/refs_vfs2_state_autogen.go b/pkg/refsvfs2/refsvfs2_state_autogen.go index 46925b4a4..ca5fbb104 100644 --- a/pkg/refs_vfs2/refs_vfs2_state_autogen.go +++ b/pkg/refsvfs2/refsvfs2_state_autogen.go @@ -1,3 +1,3 @@ // automatically generated by stateify. -package refs_vfs2 +package refsvfs2 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 { diff --git a/pkg/sentry/kernel/abstract_socket_namespace.go b/pkg/sentry/kernel/abstract_socket_namespace.go index 1b9721534..0ddbe5ff6 100644 --- a/pkg/sentry/kernel/abstract_socket_namespace.go +++ b/pkg/sentry/kernel/abstract_socket_namespace.go @@ -19,7 +19,7 @@ import ( "syscall" "gvisor.dev/gvisor/pkg/context" - "gvisor.dev/gvisor/pkg/refs_vfs2" + "gvisor.dev/gvisor/pkg/refsvfs2" "gvisor.dev/gvisor/pkg/sentry/socket/unix/transport" "gvisor.dev/gvisor/pkg/sync" ) @@ -27,7 +27,7 @@ import ( // +stateify savable type abstractEndpoint struct { ep transport.BoundEndpoint - socket refs_vfs2.RefCounter + socket refsvfs2.RefCounter name string ns *AbstractSocketNamespace } @@ -57,7 +57,7 @@ func NewAbstractSocketNamespace() *AbstractSocketNamespace { // its backing socket. type boundEndpoint struct { transport.BoundEndpoint - socket refs_vfs2.RefCounter + socket refsvfs2.RefCounter } // Release implements transport.BoundEndpoint.Release. @@ -89,7 +89,7 @@ func (a *AbstractSocketNamespace) BoundEndpoint(name string) transport.BoundEndp // // When the last reference managed by socket is dropped, ep may be removed from the // namespace. -func (a *AbstractSocketNamespace) Bind(ctx context.Context, name string, ep transport.BoundEndpoint, socket refs_vfs2.RefCounter) error { +func (a *AbstractSocketNamespace) Bind(ctx context.Context, name string, ep transport.BoundEndpoint, socket refsvfs2.RefCounter) error { a.mu.Lock() defer a.mu.Unlock() @@ -109,7 +109,7 @@ func (a *AbstractSocketNamespace) Bind(ctx context.Context, name string, ep tran // Remove removes the specified socket at name from the abstract socket // namespace, if it has not yet been replaced. -func (a *AbstractSocketNamespace) Remove(name string, socket refs_vfs2.RefCounter) { +func (a *AbstractSocketNamespace) Remove(name string, socket refsvfs2.RefCounter) { a.mu.Lock() defer a.mu.Unlock() diff --git a/pkg/sentry/kernel/fd_table.go b/pkg/sentry/kernel/fd_table.go index 0ec7344cd..81a998966 100644 --- a/pkg/sentry/kernel/fd_table.go +++ b/pkg/sentry/kernel/fd_table.go @@ -110,7 +110,7 @@ func (f *FDTable) saveDescriptorTable() map[int32]descriptor { func (f *FDTable) loadDescriptorTable(m map[int32]descriptor) { ctx := context.Background() - f.init() // Initialize table. + f.initNoLeakCheck() // Initialize table. f.used = 0 for fd, d := range m { if file, fileVFS2 := f.setAll(ctx, fd, d.file, d.fileVFS2, d.flags); file != nil || fileVFS2 != nil { diff --git a/pkg/sentry/kernel/fd_table_refs.go b/pkg/sentry/kernel/fd_table_refs.go index ecba138ac..cbf2e85ed 100644 --- a/pkg/sentry/kernel/fd_table_refs.go +++ b/pkg/sentry/kernel/fd_table_refs.go @@ -2,11 +2,9 @@ package kernel 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 FDTableownerType *FDTable // 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 FDTableRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type FDTableRefs struct { refCount int64 } -func (r *FDTableRefs) 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, FDTableownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *FDTableRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", FDTableownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *FDTableRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*FDTableRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *FDTableRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", FDTableownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *FDTableRefs) ReadRefs() int64 { //go:nosplit func (r *FDTableRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, FDTableownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, FDTableownerType)) } } @@ -110,9 +95,18 @@ func (r *FDTableRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, FDTableownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", FDTableownerType)) + } if destroy != nil { destroy() } } } + +func (r *FDTableRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/kernel/fd_table_unsafe.go b/pkg/sentry/kernel/fd_table_unsafe.go index da79e6627..3476551f3 100644 --- a/pkg/sentry/kernel/fd_table_unsafe.go +++ b/pkg/sentry/kernel/fd_table_unsafe.go @@ -31,14 +31,21 @@ type descriptorTable struct { slice unsafe.Pointer `state:".(map[int32]*descriptor)"` } -// init initializes the table. +// initNoLeakCheck initializes the table without enabling leak checking. // -// TODO(gvisor.dev/1486): Enable leak check for FDTable. -func (f *FDTable) init() { +// This is used when loading an FDTable after S/R, during which the ref count +// object itself will enable leak checking if necessary. +func (f *FDTable) initNoLeakCheck() { var slice []unsafe.Pointer // Empty slice. atomic.StorePointer(&f.slice, unsafe.Pointer(&slice)) } +// init initializes the table with leak checking. +func (f *FDTable) init() { + f.initNoLeakCheck() + f.EnableLeakCheck() +} + // get gets a file entry. // // The boolean indicates whether this was in range. diff --git a/pkg/sentry/kernel/fs_context.go b/pkg/sentry/kernel/fs_context.go index 08ea2e09c..41fb2a784 100644 --- a/pkg/sentry/kernel/fs_context.go +++ b/pkg/sentry/kernel/fs_context.go @@ -130,13 +130,15 @@ func (f *FSContext) Fork() *FSContext { f.root.IncRef() } - return &FSContext{ + ctx := &FSContext{ cwd: f.cwd, root: f.root, cwdVFS2: f.cwdVFS2, rootVFS2: f.rootVFS2, umask: f.umask, } + ctx.EnableLeakCheck() + return ctx } // WorkingDirectory returns the current working directory. diff --git a/pkg/sentry/kernel/fs_context_refs.go b/pkg/sentry/kernel/fs_context_refs.go index fb2fde971..025f11faa 100644 --- a/pkg/sentry/kernel/fs_context_refs.go +++ b/pkg/sentry/kernel/fs_context_refs.go @@ -2,11 +2,9 @@ package kernel 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 FSContextownerType *FSContext // 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 FSContextRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type FSContextRefs struct { refCount int64 } -func (r *FSContextRefs) 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, FSContextownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *FSContextRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", FSContextownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *FSContextRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*FSContextRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *FSContextRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", FSContextownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *FSContextRefs) ReadRefs() int64 { //go:nosplit func (r *FSContextRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, FSContextownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, FSContextownerType)) } } @@ -110,9 +95,18 @@ func (r *FSContextRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, FSContextownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", FSContextownerType)) + } if destroy != nil { destroy() } } } + +func (r *FSContextRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/kernel/ipc_namespace.go b/pkg/sentry/kernel/ipc_namespace.go index 3f34ee0db..b87e40dd1 100644 --- a/pkg/sentry/kernel/ipc_namespace.go +++ b/pkg/sentry/kernel/ipc_namespace.go @@ -55,7 +55,7 @@ func (i *IPCNamespace) ShmRegistry() *shm.Registry { return i.shms } -// DecRef implements refs_vfs2.RefCounter.DecRef. +// DecRef implements refsvfs2.RefCounter.DecRef. func (i *IPCNamespace) DecRef(ctx context.Context) { i.IPCNamespaceRefs.DecRef(func() { i.shms.Release(ctx) diff --git a/pkg/sentry/kernel/ipc_namespace_refs.go b/pkg/sentry/kernel/ipc_namespace_refs.go index 263919299..aec0f7a41 100644 --- a/pkg/sentry/kernel/ipc_namespace_refs.go +++ b/pkg/sentry/kernel/ipc_namespace_refs.go @@ -2,11 +2,9 @@ package kernel 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 IPCNamespaceownerType *IPCNamespace // 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 IPCNamespaceRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type IPCNamespaceRefs struct { refCount int64 } -func (r *IPCNamespaceRefs) 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, IPCNamespaceownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *IPCNamespaceRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", IPCNamespaceownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *IPCNamespaceRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*IPCNamespaceRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *IPCNamespaceRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", IPCNamespaceownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *IPCNamespaceRefs) ReadRefs() int64 { //go:nosplit func (r *IPCNamespaceRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, IPCNamespaceownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, IPCNamespaceownerType)) } } @@ -110,9 +95,18 @@ func (r *IPCNamespaceRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, IPCNamespaceownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", IPCNamespaceownerType)) + } if destroy != nil { destroy() } } } + +func (r *IPCNamespaceRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/kernel/kernel_state_autogen.go b/pkg/sentry/kernel/kernel_state_autogen.go index b7d6d4f1c..be3c71199 100644 --- a/pkg/sentry/kernel/kernel_state_autogen.go +++ b/pkg/sentry/kernel/kernel_state_autogen.go @@ -169,10 +169,9 @@ func (r *FDTableRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *FDTableRefs) afterLoad() {} - func (r *FDTableRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (f *FSContext) StateTypeName() string { @@ -230,10 +229,9 @@ func (r *FSContextRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *FSContextRefs) afterLoad() {} - func (r *FSContextRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (i *IPCNamespace) StateTypeName() string { @@ -285,10 +283,9 @@ func (r *IPCNamespaceRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *IPCNamespaceRefs) afterLoad() {} - func (r *IPCNamespaceRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (k *Kernel) StateTypeName() string { @@ -758,10 +755,9 @@ func (r *ProcessGroupRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *ProcessGroupRefs) afterLoad() {} - func (r *ProcessGroupRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (p *ptraceOptions) StateTypeName() string { @@ -932,10 +928,9 @@ func (r *SessionRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *SessionRefs) afterLoad() {} - func (r *SessionRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (s *Session) StateTypeName() string { diff --git a/pkg/sentry/kernel/process_group_refs.go b/pkg/sentry/kernel/process_group_refs.go index 4ed6e6458..1f4486817 100644 --- a/pkg/sentry/kernel/process_group_refs.go +++ b/pkg/sentry/kernel/process_group_refs.go @@ -2,11 +2,9 @@ package kernel 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 ProcessGroupownerType *ProcessGroup // 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 ProcessGroupRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type ProcessGroupRefs struct { refCount int64 } -func (r *ProcessGroupRefs) 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, ProcessGroupownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *ProcessGroupRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", ProcessGroupownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *ProcessGroupRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*ProcessGroupRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *ProcessGroupRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", ProcessGroupownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *ProcessGroupRefs) ReadRefs() int64 { //go:nosplit func (r *ProcessGroupRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, ProcessGroupownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, ProcessGroupownerType)) } } @@ -110,9 +95,18 @@ func (r *ProcessGroupRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, ProcessGroupownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", ProcessGroupownerType)) + } if destroy != nil { destroy() } } } + +func (r *ProcessGroupRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/kernel/session_refs.go b/pkg/sentry/kernel/session_refs.go index f2e1bb797..86df93be1 100644 --- a/pkg/sentry/kernel/session_refs.go +++ b/pkg/sentry/kernel/session_refs.go @@ -2,11 +2,9 @@ package kernel 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 SessionownerType *Session // 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 SessionRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type SessionRefs struct { refCount int64 } -func (r *SessionRefs) 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, SessionownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *SessionRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", SessionownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *SessionRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*SessionRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *SessionRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", SessionownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *SessionRefs) ReadRefs() int64 { //go:nosplit func (r *SessionRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, SessionownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, SessionownerType)) } } @@ -110,9 +95,18 @@ func (r *SessionRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, SessionownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", SessionownerType)) + } if destroy != nil { destroy() } } } + +func (r *SessionRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/kernel/shm/shm_refs.go b/pkg/sentry/kernel/shm/shm_refs.go index 51e07d0b3..58b0e80cf 100644 --- a/pkg/sentry/kernel/shm/shm_refs.go +++ b/pkg/sentry/kernel/shm/shm_refs.go @@ -2,11 +2,9 @@ package shm 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 ShmownerType *Shm // 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 ShmRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type ShmRefs struct { refCount int64 } -func (r *ShmRefs) 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, ShmownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *ShmRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", ShmownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *ShmRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*ShmRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *ShmRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", ShmownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *ShmRefs) ReadRefs() int64 { //go:nosplit func (r *ShmRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, ShmownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, ShmownerType)) } } @@ -110,9 +95,18 @@ func (r *ShmRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, ShmownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", ShmownerType)) + } if destroy != nil { destroy() } } } + +func (r *ShmRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/kernel/shm/shm_state_autogen.go b/pkg/sentry/kernel/shm/shm_state_autogen.go index 8202c37d6..aca4c9b96 100644 --- a/pkg/sentry/kernel/shm/shm_state_autogen.go +++ b/pkg/sentry/kernel/shm/shm_state_autogen.go @@ -129,10 +129,9 @@ func (r *ShmRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *ShmRefs) afterLoad() {} - func (r *ShmRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func init() { diff --git a/pkg/sentry/mm/aio_mappable_refs.go b/pkg/sentry/mm/aio_mappable_refs.go index b99909f07..6a8c753ed 100644 --- a/pkg/sentry/mm/aio_mappable_refs.go +++ b/pkg/sentry/mm/aio_mappable_refs.go @@ -2,11 +2,9 @@ package mm 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 aioMappableownerType *aioMappable // 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 aioMappableRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type aioMappableRefs struct { refCount int64 } -func (r *aioMappableRefs) 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, aioMappableownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *aioMappableRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", aioMappableownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *aioMappableRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*aioMappableRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *aioMappableRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", aioMappableownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *aioMappableRefs) ReadRefs() int64 { //go:nosplit func (r *aioMappableRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, aioMappableownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, aioMappableownerType)) } } @@ -110,9 +95,18 @@ func (r *aioMappableRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, aioMappableownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", aioMappableownerType)) + } if destroy != nil { destroy() } } } + +func (r *aioMappableRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/mm/mm_state_autogen.go b/pkg/sentry/mm/mm_state_autogen.go index 78d8fe7f9..a2d098d30 100644 --- a/pkg/sentry/mm/mm_state_autogen.go +++ b/pkg/sentry/mm/mm_state_autogen.go @@ -132,10 +132,9 @@ func (r *aioMappableRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *aioMappableRefs) afterLoad() {} - func (r *aioMappableRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (s *fileRefcountSet) StateTypeName() string { @@ -637,10 +636,9 @@ func (r *SpecialMappableRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *SpecialMappableRefs) afterLoad() {} - func (r *SpecialMappableRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (s *vmaSet) StateTypeName() string { diff --git a/pkg/sentry/mm/special_mappable_refs.go b/pkg/sentry/mm/special_mappable_refs.go index 035bbe690..aa75939ea 100644 --- a/pkg/sentry/mm/special_mappable_refs.go +++ b/pkg/sentry/mm/special_mappable_refs.go @@ -2,11 +2,9 @@ package mm 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 SpecialMappableownerType *SpecialMappable // 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 SpecialMappableRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type SpecialMappableRefs struct { refCount int64 } -func (r *SpecialMappableRefs) 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, SpecialMappableownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *SpecialMappableRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", SpecialMappableownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *SpecialMappableRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*SpecialMappableRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *SpecialMappableRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", SpecialMappableownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *SpecialMappableRefs) ReadRefs() int64 { //go:nosplit func (r *SpecialMappableRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, SpecialMappableownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, SpecialMappableownerType)) } } @@ -110,9 +95,18 @@ func (r *SpecialMappableRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, SpecialMappableownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", SpecialMappableownerType)) + } if destroy != nil { destroy() } } } + +func (r *SpecialMappableRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/socket/unix/socket_refs.go b/pkg/sentry/socket/unix/socket_refs.go index ea63dc659..45b7c77d5 100644 --- a/pkg/sentry/socket/unix/socket_refs.go +++ b/pkg/sentry/socket/unix/socket_refs.go @@ -2,11 +2,9 @@ package unix 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 socketOperationsownerType *SocketOperations // 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 socketOperationsRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type socketOperationsRefs struct { refCount int64 } -func (r *socketOperationsRefs) 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, socketOperationsownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *socketOperationsRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", socketOperationsownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *socketOperationsRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*socketOperationsRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *socketOperationsRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", socketOperationsownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *socketOperationsRefs) ReadRefs() int64 { //go:nosplit func (r *socketOperationsRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, socketOperationsownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, socketOperationsownerType)) } } @@ -110,9 +95,18 @@ func (r *socketOperationsRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, socketOperationsownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", socketOperationsownerType)) + } if destroy != nil { destroy() } } } + +func (r *socketOperationsRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/socket/unix/socket_vfs2_refs.go b/pkg/sentry/socket/unix/socket_vfs2_refs.go index dc55f2947..479dd5ef0 100644 --- a/pkg/sentry/socket/unix/socket_vfs2_refs.go +++ b/pkg/sentry/socket/unix/socket_vfs2_refs.go @@ -2,11 +2,9 @@ package unix 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 socketVFS2ownerType *SocketVFS2 // 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 socketVFS2Refs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type socketVFS2Refs struct { refCount int64 } -func (r *socketVFS2Refs) 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, socketVFS2ownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *socketVFS2Refs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", socketVFS2ownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *socketVFS2Refs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*socketVFS2Refs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *socketVFS2Refs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", socketVFS2ownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *socketVFS2Refs) ReadRefs() int64 { //go:nosplit func (r *socketVFS2Refs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, socketVFS2ownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, socketVFS2ownerType)) } } @@ -110,9 +95,18 @@ func (r *socketVFS2Refs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, socketVFS2ownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", socketVFS2ownerType)) + } if destroy != nil { destroy() } } } + +func (r *socketVFS2Refs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/socket/unix/transport/queue_refs.go b/pkg/sentry/socket/unix/transport/queue_refs.go index 0d4e34988..de3bb9270 100644 --- a/pkg/sentry/socket/unix/transport/queue_refs.go +++ b/pkg/sentry/socket/unix/transport/queue_refs.go @@ -2,11 +2,9 @@ package transport 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 queueownerType *queue // 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 queueRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type queueRefs struct { refCount int64 } -func (r *queueRefs) 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, queueownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *queueRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", queueownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *queueRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*queueRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *queueRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", queueownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *queueRefs) ReadRefs() int64 { //go:nosplit func (r *queueRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, queueownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, queueownerType)) } } @@ -110,9 +95,18 @@ func (r *queueRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, queueownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", queueownerType)) + } if destroy != nil { destroy() } } } + +func (r *queueRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/socket/unix/transport/transport_state_autogen.go b/pkg/sentry/socket/unix/transport/transport_state_autogen.go index 015fbc90a..4ec849a7f 100644 --- a/pkg/sentry/socket/unix/transport/transport_state_autogen.go +++ b/pkg/sentry/socket/unix/transport/transport_state_autogen.go @@ -126,10 +126,9 @@ func (r *queueRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *queueRefs) afterLoad() {} - func (r *queueRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (l *messageList) StateTypeName() string { diff --git a/pkg/sentry/socket/unix/unix.go b/pkg/sentry/socket/unix/unix.go index a4a76d0a3..adad485a9 100644 --- a/pkg/sentry/socket/unix/unix.go +++ b/pkg/sentry/socket/unix/unix.go @@ -81,7 +81,6 @@ func NewWithDirent(ctx context.Context, d *fs.Dirent, ep transport.Endpoint, sty }, } s.EnableLeakCheck() - return fs.NewFile(ctx, d, flags, &s) } diff --git a/pkg/sentry/socket/unix/unix_state_autogen.go b/pkg/sentry/socket/unix/unix_state_autogen.go index 6b2886e4f..fba990d9a 100644 --- a/pkg/sentry/socket/unix/unix_state_autogen.go +++ b/pkg/sentry/socket/unix/unix_state_autogen.go @@ -23,10 +23,9 @@ func (r *socketOperationsRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *socketOperationsRefs) afterLoad() {} - func (r *socketOperationsRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (r *socketVFS2Refs) StateTypeName() string { @@ -46,10 +45,9 @@ func (r *socketVFS2Refs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *socketVFS2Refs) afterLoad() {} - func (r *socketVFS2Refs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (s *SocketOperations) StateTypeName() string { diff --git a/pkg/sentry/socket/unix/unix_vfs2.go b/pkg/sentry/socket/unix/unix_vfs2.go index 678355fb9..21514c7bb 100644 --- a/pkg/sentry/socket/unix/unix_vfs2.go +++ b/pkg/sentry/socket/unix/unix_vfs2.go @@ -80,6 +80,7 @@ func NewFileDescription(ep transport.Endpoint, stype linux.SockType, flags uint3 stype: stype, }, } + sock.EnableLeakCheck() sock.LockFD.Init(locks) vfsfd := &sock.vfsfd if err := vfsfd.Init(sock, flags, mnt, d, &vfs.FileDescriptionOptions{ diff --git a/pkg/sentry/vfs/file_description_refs.go b/pkg/sentry/vfs/file_description_refs.go index bdd7e6554..951cfbd4d 100644 --- a/pkg/sentry/vfs/file_description_refs.go +++ b/pkg/sentry/vfs/file_description_refs.go @@ -2,11 +2,9 @@ package vfs 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 FileDescriptionownerType *FileDescription // 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 FileDescriptionRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type FileDescriptionRefs struct { refCount int64 } -func (r *FileDescriptionRefs) 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, FileDescriptionownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *FileDescriptionRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", FileDescriptionownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *FileDescriptionRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*FileDescriptionRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *FileDescriptionRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", FileDescriptionownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *FileDescriptionRefs) ReadRefs() int64 { //go:nosplit func (r *FileDescriptionRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, FileDescriptionownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, FileDescriptionownerType)) } } @@ -110,9 +95,18 @@ func (r *FileDescriptionRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, FileDescriptionownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", FileDescriptionownerType)) + } if destroy != nil { destroy() } } } + +func (r *FileDescriptionRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/vfs/filesystem_refs.go b/pkg/sentry/vfs/filesystem_refs.go index 38a9a986f..f1abc120d 100644 --- a/pkg/sentry/vfs/filesystem_refs.go +++ b/pkg/sentry/vfs/filesystem_refs.go @@ -2,11 +2,9 @@ package vfs 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 FilesystemownerType *Filesystem // 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 FilesystemRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type FilesystemRefs struct { refCount int64 } -func (r *FilesystemRefs) 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, FilesystemownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *FilesystemRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", FilesystemownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *FilesystemRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*FilesystemRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *FilesystemRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", FilesystemownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *FilesystemRefs) ReadRefs() int64 { //go:nosplit func (r *FilesystemRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, FilesystemownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, FilesystemownerType)) } } @@ -110,9 +95,18 @@ func (r *FilesystemRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, FilesystemownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", FilesystemownerType)) + } if destroy != nil { destroy() } } } + +func (r *FilesystemRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/vfs/mount_namespace_refs.go b/pkg/sentry/vfs/mount_namespace_refs.go index 63285fb8e..32e28ebf8 100644 --- a/pkg/sentry/vfs/mount_namespace_refs.go +++ b/pkg/sentry/vfs/mount_namespace_refs.go @@ -2,11 +2,9 @@ package vfs 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 MountNamespaceownerType *MountNamespace // 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 MountNamespaceRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type MountNamespaceRefs struct { refCount int64 } -func (r *MountNamespaceRefs) 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, MountNamespaceownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *MountNamespaceRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", MountNamespaceownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *MountNamespaceRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*MountNamespaceRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *MountNamespaceRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", MountNamespaceownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *MountNamespaceRefs) ReadRefs() int64 { //go:nosplit func (r *MountNamespaceRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, MountNamespaceownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, MountNamespaceownerType)) } } @@ -110,9 +95,18 @@ func (r *MountNamespaceRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, MountNamespaceownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", MountNamespaceownerType)) + } if destroy != nil { destroy() } } } + +func (r *MountNamespaceRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/sentry/vfs/vfs_state_autogen.go b/pkg/sentry/vfs/vfs_state_autogen.go index d78221080..03c84829d 100644 --- a/pkg/sentry/vfs/vfs_state_autogen.go +++ b/pkg/sentry/vfs/vfs_state_autogen.go @@ -690,10 +690,9 @@ func (r *FileDescriptionRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *FileDescriptionRefs) afterLoad() {} - func (r *FileDescriptionRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (fs *Filesystem) StateTypeName() string { @@ -802,10 +801,9 @@ func (r *FilesystemRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *FilesystemRefs) afterLoad() {} - func (r *FilesystemRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (r *registeredFilesystemType) StateTypeName() string { @@ -1169,10 +1167,9 @@ func (r *MountNamespaceRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *MountNamespaceRefs) afterLoad() {} - func (r *MountNamespaceRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func (g *GetDentryOptions) StateTypeName() string { diff --git a/pkg/tcpip/link/tun/device.go b/pkg/tcpip/link/tun/device.go index f94491026..cda6328a2 100644 --- a/pkg/tcpip/link/tun/device.go +++ b/pkg/tcpip/link/tun/device.go @@ -150,7 +150,6 @@ func attachOrCreateNIC(s *stack.Stack, name, prefix string, linkCaps stack.LinkE // 2. Creating a new NIC. id := tcpip.NICID(s.UniqueID()) - // TODO(gvisor.dev/1486): enable leak check for tunEndpoint. endpoint := &tunEndpoint{ Endpoint: channel.New(defaultDevOutQueueLen, defaultDevMtu, ""), stack: s, @@ -158,6 +157,7 @@ func attachOrCreateNIC(s *stack.Stack, name, prefix string, linkCaps stack.LinkE name: name, isTap: prefix == "tap", } + endpoint.EnableLeakCheck() endpoint.Endpoint.LinkEPCapabilities = linkCaps if endpoint.name == "" { endpoint.name = fmt.Sprintf("%s%d", prefix, id) diff --git a/pkg/tcpip/link/tun/tun_endpoint_refs.go b/pkg/tcpip/link/tun/tun_endpoint_refs.go index e0595429c..7ca1ace61 100644 --- a/pkg/tcpip/link/tun/tun_endpoint_refs.go +++ b/pkg/tcpip/link/tun/tun_endpoint_refs.go @@ -2,11 +2,9 @@ package tun 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 tunEndpointownerType *tunEndpoint // 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 tunEndpointRefs struct { // refCount is composed of two fields: @@ -36,24 +29,16 @@ type tunEndpointRefs struct { refCount int64 } -func (r *tunEndpointRefs) 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, tunEndpointownerType, n) +// EnableLeakCheck enables reference leak checking on r. +func (r *tunEndpointRefs) EnableLeakCheck() { + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Register(r, fmt.Sprintf("%T", tunEndpointownerType)) } } -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *tunEndpointRefs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*tunEndpointRefs).finalize) - } +// LeakMessage implements refsvfs2.CheckedObject.LeakMessage. +func (r *tunEndpointRefs) LeakMessage() string { + return fmt.Sprintf("%T %p: reference count of %d instead of 0", tunEndpointownerType, r, r.ReadRefs()) } // ReadRefs returns the current number of references. The returned count is @@ -68,7 +53,7 @@ func (r *tunEndpointRefs) ReadRefs() int64 { //go:nosplit func (r *tunEndpointRefs) IncRef() { if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, tunEndpointownerType)) + panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, tunEndpointownerType)) } } @@ -110,9 +95,18 @@ func (r *tunEndpointRefs) DecRef(destroy func()) { panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, tunEndpointownerType)) case v == -1: + if refsvfs2.LeakCheckEnabled() { + refsvfs2.Unregister(r, fmt.Sprintf("%T", tunEndpointownerType)) + } if destroy != nil { destroy() } } } + +func (r *tunEndpointRefs) afterLoad() { + if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 { + r.EnableLeakCheck() + } +} diff --git a/pkg/tcpip/link/tun/tun_state_autogen.go b/pkg/tcpip/link/tun/tun_state_autogen.go index 165b50835..3515d86fd 100644 --- a/pkg/tcpip/link/tun/tun_state_autogen.go +++ b/pkg/tcpip/link/tun/tun_state_autogen.go @@ -53,10 +53,9 @@ func (r *tunEndpointRefs) StateSave(stateSinkObject state.Sink) { stateSinkObject.Save(0, &r.refCount) } -func (r *tunEndpointRefs) afterLoad() {} - func (r *tunEndpointRefs) StateLoad(stateSourceObject state.Source) { stateSourceObject.Load(0, &r.refCount) + stateSourceObject.AfterLoad(r.afterLoad) } func init() { diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index a971d20ec..8c6ab213d 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -35,6 +35,7 @@ import ( "gvisor.dev/gvisor/pkg/memutil" "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/control" "gvisor.dev/gvisor/pkg/sentry/fdimport" @@ -476,6 +477,12 @@ func (l *Loader) Destroy() { // save/restore. l.k.Release() + // All sentry-created resources should have been released at this point; + // check for reference leaks. + if refsvfs2.LeakCheckEnabled() { + refsvfs2.DoLeakCheck() + } + // In the success case, stdioFDs and goferFDs will only contain // released/closed FDs that ownership has been passed over to host FDs and // gofer sessions. Close them here in case of failure. |