Skip to content

Commit

Permalink
internal/configstore: do not require console for subprocess in windows
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
  • Loading branch information
hyangah committed May 30, 2024
1 parent 39ace7a commit acd60de
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
5 changes: 5 additions & 0 deletions internal/configstore/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
33 changes: 33 additions & 0 deletions internal/configstore/download_windows.go
Original file line number Diff line number Diff line change
@@ -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,
}
}

0 comments on commit acd60de

Please sign in to comment.