runsc: Don't close the control server in a defer.

Closing the control server will block until all open requests have completed.
If a control server method panics, we end up stuck because the defer'd Destroy
function will never return.

PiperOrigin-RevId: 204354676
Change-Id: I6bb1d84b31242d7c3f20d5334b1c966bd6a61dbf

2 files changed, 6 insertions, 4 deletions

diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go
index a3cc0e4a4..7de0a84cc 100644
--- a/runsc/boot/loader.go
+++ b/runsc/boot/loader.go
@@ -347,9 +347,12 @@ func newProcess(spec *specs.Spec, conf *Config, ioFDs []int, console bool, creds
 }
 
 // Destroy cleans up all resources used by the loader.
+//
+// Note that this will block until all open control server connections have
+// been closed. For that reason, this should NOT be called in a defer, because
+// a panic in a control server rpc would then hang forever.
 func (l *Loader) Destroy() {
 	if l.ctrl != nil {
-		// Shut down control server.
 		l.ctrl.srv.Stop()
 	}
 	l.stopSignalForwarding()
diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go
index b19da315f..70c4616b4 100644
--- a/runsc/cmd/boot.go
+++ b/runsc/cmd/boot.go
@@ -142,24 +142,23 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
 	}
 
 	// Create the loader.
-
 	l, err := boot.New(spec, conf, b.controllerFD, b.restoreFD, b.ioFDs.GetArray(), b.console)
-
 	if err != nil {
 		Fatalf("error creating loader: %v", err)
 	}
-	defer l.Destroy()
 
 	// Wait for the start signal from runsc.
 	l.WaitForStartSignal()
 
 	// Run the application and wait for it to finish.
 	if err := l.Run(); err != nil {
+		l.Destroy()
 		Fatalf("error running sandbox: %v", err)
 	}
 
 	ws := l.WaitExit()
 	log.Infof("application exiting with %+v", ws)
 	*waitStatus = syscall.WaitStatus(ws.Status())
+	l.Destroy()
 	return subcommands.ExitSuccess
 }