Skip to content

Commit

Permalink
Allocate a conhost during Windows service startup
Browse files Browse the repository at this point in the history
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 <kevpar@microsoft.com>
  • Loading branch information
kevpar committed Jul 25, 2019
1 parent fdab4f4 commit 1c7eab1
Showing 1 changed file with 20 additions and 2 deletions.
22 changes: 20 additions & 2 deletions cmd/containerd/command/service_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
}
Expand All @@ -341,7 +360,6 @@ func registerUnregisterService(root string) (bool, error) {

logrus.AddHook(&etwHook{log})
logrus.SetOutput(ioutil.Discard)

}
return false, nil
}
Expand Down

0 comments on commit 1c7eab1

Please sign in to comment.