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

Jump to bottom

os: ErrProcessDone not defined on windows #42311

Closed
qmuntal opened this issue Oct 31, 2020 · 4 comments
Closed

os: ErrProcessDone not defined on windows #42311

qmuntal opened this issue Oct 31, 2020 · 4 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Windows release-blocker
Milestone
Go1.16

Comments

@qmuntal
Copy link
Contributor

qmuntal commented Oct 31, 2020 •
edited by dmitshur

What version of Go are you using (go version)?

$ go tip

Does this issue reproduce with the latest release?

It reproduces on tip

What operating system and processor architecture are you using (go env)?

go env Output
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Dante\AppData\Local\go-build
set GOENV=C:\Users\Dante\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Dante\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Dante\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Users\Dante\Documents\Code\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Users\Dante\Documents\Code\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\Dante\Documents\Code\helloworld\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\Dante\AppData\Local\Temp\go-build704363910=/tmp/go-build -gno-record-gcc-switches

What did you do?

Use the error os.ErrProcessDone with GOOS=windows, which has just been exported in tip CL 242998.

Playground link: https://play.golang.org/p/OMXQb0DeySn

What did you expect to see?

Code compiling and p.Signal() returning os.ErrProcessDone

What did you see instead?

.\main.go:23:13: undefined: os.ErrProcessDone

It seems that os.ErrProcessDone has only been exported for Unix, even though this same error can also be returned on Windows (see os/exec_windows.go#L64).
It was probably forgotten in CL 242998 because the error was declared as a sentinel error on Unix and inlined on Windows.
@qmuntal
Copy link
Contributor Author

qmuntal commented Oct 31, 2020

I´ve just found that os.ErrProcessDone could also be used on plan9 (see os/exec_plan9.go#L55).

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Oct 31, 2020
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Oct 31, 2020
@alwindoss
Copy link
Contributor

I see that in os/exec_unix.go method signal returns ErrProcessDone but the same function in os/exec_windows.go returns errors.New("os: process already finished").

Is the solution to this issue as simple as exporting the variable ErrProcessDone in os/exec_windows.go and returning this error in appropriate functions?

@alwindoss alwindoss mentioned this issue Oct 31, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/266997 mentions this issue: os: export ErrProcessDone variable in windows and plan9

@dmitshur dmitshur modified the milestones: Backlog, Go1.17, Go1.16 Nov 1, 2020
@bcmills
Copy link
Contributor

bcmills commented Nov 2, 2020

Marking as release-blocker because this is a defect in a new API being added in Go 1.16 (#39444).

@golang golang locked and limited conversation to collaborators Nov 3, 2021
gopherbot pushed a commit that referenced this issue Apr 3, 2022
@iwdgo @ianlancetaylor
os: add handling of os.Interrupt for windows
3451844 
Add GenerateConsoleCtrlEvent call to internal syscall package.
Define ErrProcessDone while reviewing handling of os.Signal().
Update test to run for windows using the added call.

Fixes #42311
Fixes #46354

Change-Id: I460955efc76c4febe04b612ac9a0670e62ba5ff3
Reviewed-on: https://go-review.googlesource.com/c/go/+/367495
Trust: Patrik Nyblom <pnyb@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@bcmills bcmills reopened this Apr 4, 2022
@bcmills bcmills closed this as completed Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Windows release-blocker
Projects
None yet
Milestone
Go1.16
Development

Successfully merging a pull request may close this issue.

os: export ErrProcessDone variable in windows and plan9
6 participants
@alwindoss @dmitshur @ianlancetaylor @qmuntal @bcmills @gopherbot