From acd60dee2c9943b5c2900e3cf4140d616e20d313 Mon Sep 17 00:00:00 2001 From: "Hana (Hyang-Ah) Kim" Date: Thu, 30 May 2024 10:35:20 -0400 Subject: [PATCH] internal/configstore: do not require console for subprocess in windows The configstore downloads the latest telemetry config using the `go mod download` command. The uploader process that calls this configstore is likely a daemonized process (see x/telemetry/start_windows.go) and has no console attached. The console creation behavior when the parent is a console process without console is not clearly documented anywhere, but empirically we observed a new console (new window) is created for the subprocess. I am not sure if this is due to the system default configuration, or due to how go's os/exec is implemented on windows. Prevent the new console creation by explicitly setting the CREATE_NO_WINDOW attribute. https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags Manually tested. For golang/go#67660 Change-Id: Ia86ebafe3d4d7bcb63a7eaf9ce01c4eb32b43809 Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/589061 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley Reviewed-by: Michael Matloob --- internal/configstore/download.go | 5 ++++ internal/configstore/download_windows.go | 33 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 internal/configstore/download_windows.go diff --git a/internal/configstore/download.go b/internal/configstore/download.go index b73763a..a38f371 100644 --- a/internal/configstore/download.go +++ b/internal/configstore/download.go @@ -25,6 +25,10 @@ const ( configFileName = "config.json" ) +// needNoConsole is used on windows to set the windows.CREATE_NO_WINDOW +// creation flag. +var needNoConsole = func(cmd *exec.Cmd) {} + // Download fetches the requested telemetry UploadConfig using "go mod // download". If envOverlay is provided, it is appended to the environment used // for invoking the go command. @@ -37,6 +41,7 @@ func Download(version string, envOverlay []string) (*telemetry.UploadConfig, str modVer := ModulePath + "@" + version var stdout, stderr bytes.Buffer cmd := exec.Command("go", "mod", "download", "-json", modVer) + needNoConsole(cmd) cmd.Env = append(os.Environ(), envOverlay...) cmd.Stdout = &stdout cmd.Stderr = &stderr diff --git a/internal/configstore/download_windows.go b/internal/configstore/download_windows.go new file mode 100644 index 0000000..1368de1 --- /dev/null +++ b/internal/configstore/download_windows.go @@ -0,0 +1,33 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build windows + +package configstore + +import ( + "os/exec" + "syscall" + + "golang.org/x/sys/windows" +) + +func init() { + needNoConsole = needNoConsoleWindows +} + +func needNoConsoleWindows(cmd *exec.Cmd) { + // The uploader main process is likely a daemonized process with no console. + // (see x/telemetry/start_windows.go) The console creation behavior when + // a parent is a console process without console is not clearly documented + // but empirically we observed the new console is created and attached to the + // subprocess in the default setup. + // + // Ensure no new console is attached to the subprocess by setting CREATE_NO_WINDOW. + // https://learn.microsoft.com/en-us/windows/console/creation-of-a-console + // https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags + cmd.SysProcAttr = &syscall.SysProcAttr{ + CreationFlags: windows.CREATE_NO_WINDOW, + } +}