Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: for every test run in go/testdata/script results in windows command prompt rapidly opens and closes #67660

Closed
Goclipse27 opened this issue May 27, 2024 · 7 comments
Assignees
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@Goclipse27
Copy link
Contributor

Go version

go version devel go1.23-377646589d Fri May 24 22:23:55 2024 +0000 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Ragul\AppData\Local\go-build
set GOENV=C:\Users\Ragul\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Ragul\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Ragul\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Users\Ragul\go-development\go
set GOSUMDB=off
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Users\Ragul\go-development\go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=devel go1.23-377646589d Fri May 24 22:23:55 2024 +0000
set GODEBUG=
set GOTELEMETRY=local
set GOTELEMETRYDIR=C:\Users\Ragul\AppData\Roaming\go\telemetry
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\Ragul\go-development\go\src\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\Ragul\AppData\Local\Temp\go-build994411011=/tmp/go-build -gno-record-gcc-switches

What did you do?

When I do run all.bat or running TestScript in package main_test( src/cmd/go/testdata/script), all those test scripts while running rapidly opens and closes Windows command prompt.

C:\Users\Ragul\go-development\go\bin\go.exe test -timeout 30s -run ^TestScript$ cmd/go

It didn't expect user to intervene rather it opens and closes on its own.

What did you see happen?

I am able to encounter this while runnning tests which invoke go or other commands as exec.Cmd.

Triage : I did git bisect and triage this further, I started seeing this behaviour happening from this commit - 328aa9e

It was fine until commit - 5be95e0.

What did you expect to see?

Tests should run fine invoking *exec.Cmd without windows command prompt/terminal being popped up. I don't know if the same happens in other OS environment too.

@Goclipse27
Copy link
Contributor Author

cc @matloob

@seankhliao seankhliao changed the title cmd/go: test : For every test run in go/testdata/script results in windows command prompt rapidly opens and closes.(Triage Done) cmd/go: for every test run in go/testdata/script results in windows command prompt rapidly opens and closes May 27, 2024
@matloob
Copy link
Contributor

matloob commented May 28, 2024

I can verify that this happens on Windows.

It's not happening on the Mac or Linux machines i tried it on.

@matloob matloob added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels May 28, 2024
@matloob matloob self-assigned this May 28, 2024
@findleyr findleyr added this to the Go1.23 milestone May 29, 2024
@findleyr
Copy link
Contributor

Per @rsc, this may affect darwin as well.
(reports tiny popups while running all.bash).

Theory: this may be related to daemonization+use of network.

@hyangah
Copy link
Contributor

hyangah commented May 30, 2024

My theory was daemonization (detached process) + invoking "go mod download" with exec.Command.
I no longer see this issue if I do one of the followings:

  1. disable upload --> which we don't want.
  2. don't daemonize --> that will make crash reporter less effective.
  3. disable go mod download for telemetry config query --> we need to implement another config distribution mechanism.
  4. when starting go mod download process from x/telemetry/internal/configstore.Download, configure SysProcAttr to have windows.CREATE_NO_WINDOW, ... --> seems easiest.

I couldn't reproduce the same issue on mac yet so I am guessing that's another issue.

@findleyr
Copy link
Contributor

Interesting. Is the problem that the go subprocess is started from a daemon?

Nice find with CREATE_NO_WINDOW.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/589061 mentions this issue: internal/configstore: do not require console for subprocess in windows

gopherbot pushed a commit to golang/telemetry that referenced this issue May 30, 2024
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@hyangah
Copy link
Contributor

hyangah commented May 31, 2024

Windows trybots passed on the https://go.dev/cl/588943 that updates vendored x/telemetry.
I also no longer observe the issue from manual testing on windows after it.
Closing it. #67617 is the issue about vendoring x/telemetry.

@hyangah hyangah closed this as completed May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
Status: Done
Development

No branches or pull requests

5 participants