From 1c7eab197999328e9fe6e9f20c273852fac7efb8 Mon Sep 17 00:00:00 2001 From: Kevin Parsons Date: Wed, 24 Jul 2019 17:50:24 -0700 Subject: [PATCH] Allocate a conhost during Windows service startup Creating a console for containerd causes it to be inherited by any child processes, which gives us performance and reliability improvements. See comment in code for more information. Another option considered here would be to invoke each child process with the DETACHED_PROCESS flag. This would save us the containerd console allocation. The difficulty of this approach would be ensuring that all process invocation points have had this flag added, and that any future invocations also use the flag. Signed-off-by: Kevin Parsons --- cmd/containerd/command/service_windows.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/cmd/containerd/command/service_windows.go b/cmd/containerd/command/service_windows.go index 78bcb5e84ef8..7b417ba57e0a 100644 --- a/cmd/containerd/command/service_windows.go +++ b/cmd/containerd/command/service_windows.go @@ -45,7 +45,9 @@ var ( unregisterServiceFlag bool runServiceFlag bool - setStdHandle = windows.NewLazySystemDLL("kernel32.dll").NewProc("SetStdHandle") + kernel32 = windows.NewLazySystemDLL("kernel32.dll") + setStdHandle = kernel32.NewProc("SetStdHandle") + allocConsole = kernel32.NewProc("AllocConsole") oldStderr windows.Handle panicFile *os.File @@ -322,6 +324,23 @@ func registerUnregisterService(root string) (bool, error) { } if runServiceFlag { + // Allocate a conhost for containerd here. We don't actually use this + // at all in containerd, but it will be inherited by any processes + // containerd executes, so they won't need to allocate their own + // conhosts. This is important for two reasons: + // - Creating a conhost slows down process launch. + // - We have seen reliability issues when launching many processes. + // Sometimes the process invocation will fail due to an error when + // creating the conhost. + // + // This needs to be done before initializing the panic file, as + // AllocConsole sets the stdio handles to point to the new conhost, + // and we want to make sure stderr goes to the panic file. + r, _, err := allocConsole.Call() + if r == 0 && err != nil { + return true, fmt.Errorf("error allocating conhost: %s", err) + } + if err := initPanicFile(filepath.Join(root, "panic.log")); err != nil { return true, err } @@ -341,7 +360,6 @@ func registerUnregisterService(root string) (bool, error) { logrus.AddHook(&etwHook{log}) logrus.SetOutput(ioutil.Discard) - } return false, nil }