diff options
author | Dean Deng <deandeng@google.com> | 2021-04-19 16:43:20 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-04-19 16:48:27 -0700 |
commit | 20b1c3c632277bd64eac4d0442bda9695f184fc9 (patch) | |
tree | b2627fb84af14531bffed6dec381e4cd76867533 | |
parent | 7bfc76d946b6c3f02fc32831ddc282ac2816d5ed (diff) |
Move runsc reference leak checking to better locations.
In the previous spot, there was a roughly 50% chance that leak checking would
actually run. Move it to the waitContainer() call on the root container, where
it is guaranteed to run before the sandbox process is terminated. Add it to
runsc/cli/main.go as well for good measure, in case the sandbox exit path does
not involve waitContainer().
PiperOrigin-RevId: 369329796
-rw-r--r-- | pkg/refsvfs2/refs_map.go | 25 | ||||
-rw-r--r-- | runsc/boot/loader.go | 12 | ||||
-rw-r--r-- | runsc/cli/BUILD | 1 | ||||
-rw-r--r-- | runsc/cli/main.go | 4 |
4 files changed, 25 insertions, 17 deletions
diff --git a/pkg/refsvfs2/refs_map.go b/pkg/refsvfs2/refs_map.go index 0472eca3f..fb8984dd6 100644 --- a/pkg/refsvfs2/refs_map.go +++ b/pkg/refsvfs2/refs_map.go @@ -112,20 +112,27 @@ func logEvent(obj CheckedObject, msg string) { log.Infof("[%s %p] %s:\n%s", obj.RefType(), obj, msg, refs_vfs1.FormatStack(refs_vfs1.RecordStack())) } +// checkOnce makes sure that leak checking is only done once. DoLeakCheck is +// called from multiple places (which may overlap) to cover different sandbox +// exit scenarios. +var checkOnce sync.Once + // 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() { if leakCheckEnabled() { - liveObjectsMu.Lock() - defer liveObjectsMu.Unlock() - leaked := len(liveObjects) - if leaked > 0 { - msg := fmt.Sprintf("Leak checking detected %d leaked objects:\n", leaked) - for obj := range liveObjects { - msg += obj.LeakMessage() + "\n" + checkOnce.Do(func() { + liveObjectsMu.Lock() + defer liveObjectsMu.Unlock() + leaked := len(liveObjects) + if leaked > 0 { + msg := fmt.Sprintf("Leak checking detected %d leaked objects:\n", leaked) + for obj := range liveObjects { + msg += obj.LeakMessage() + "\n" + } + log.Warningf(msg) } - log.Warningf(msg) - } + }) } } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 5d6e67279..798c1a7a7 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -492,10 +492,6 @@ func (l *Loader) Destroy() { // save/restore. l.k.Release() - // All sentry-created resources should have been released at this point; - // check for reference leaks. - 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. @@ -1002,10 +998,12 @@ func (l *Loader) waitContainer(cid string, waitStatus *uint32) error { ws := l.wait(tg) *waitStatus = ws - // Write coverage report after the root container has exited. This guarantees - // that the report is written in cases where the sandbox is killed by a signal - // after the ContainerWait request is completed. + // Check for leaks and write coverage report after the root container has + // exited. This guarantees that the report is written in cases where the + // sandbox is killed by a signal after the ContainerWait request is completed. if l.root.procArgs.ContainerID == cid { + // All sentry-created resources should have been released at this point. + refsvfs2.DoLeakCheck() coverage.Report() } return nil diff --git a/runsc/cli/BUILD b/runsc/cli/BUILD index 705738aef..360e3cea6 100644 --- a/runsc/cli/BUILD +++ b/runsc/cli/BUILD @@ -13,6 +13,7 @@ go_library( "//pkg/coverage", "//pkg/log", "//pkg/refs", + "//pkg/refsvfs2", "//pkg/sentry/platform", "//runsc/cmd", "//runsc/config", diff --git a/runsc/cli/main.go b/runsc/cli/main.go index 79eb85cff..76184cd9c 100644 --- a/runsc/cli/main.go +++ b/runsc/cli/main.go @@ -30,6 +30,7 @@ import ( "gvisor.dev/gvisor/pkg/coverage" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/refsvfs2" "gvisor.dev/gvisor/pkg/sentry/platform" "gvisor.dev/gvisor/runsc/cmd" "gvisor.dev/gvisor/runsc/config" @@ -240,7 +241,8 @@ func Main(version string) { // Call the subcommand and pass in the configuration. var ws unix.WaitStatus subcmdCode := subcommands.Execute(context.Background(), conf, &ws) - // Write coverage report before os.Exit(). + // Check for leaks and write coverage report before os.Exit(). + refsvfs2.DoLeakCheck() coverage.Report() if subcmdCode == subcommands.ExitSuccess { log.Infof("Exiting with status: %v", ws) |