Do you know whether this has ever worked? That is, do you know whether it works with older releases of Go? Thanks.

Does https://golang.org/cl/316269 fix this issue?

@AkarinVS thank you for reporting the issue. I can reproduce it on Windows 7.

I googled and I found other developers already encountered similar issue. 

https://bugzilla.mozilla.org/show_bug.cgi?id=1460995

bazelbuild/bazel#8676

https://github.com/bazelbuild/bazel/pull/8793/files

https://github.com/rprichard/win32-console-docs/blob/master/src/HandleTests/CreateProcess_InheritList.cc

It appears that PROC_THREAD_ATTRIBUTE_HANDLE_LIST cannot be used with console handles on Windows 7.

I am not sure what to do here.

We can, probably, use old pre-PROC_THREAD_ATTRIBUTE_HANDLE_LIST code for this situation. But what do we do about new features that are not supported without PROC_THREAD_ATTRIBUTE_HANDLE_LIST? And that would require restoring some of our before-PROC_THREAD_ATTRIBUTE_HANDLE_LIST code.

If we decide to restore our old CreateProcess code, perhaps we could leave current version of CreateProcess as is. But, if it returns ERROR_NO_SYSTEM_RESOURCES, then call old version of CreateProcess code. This should not punish majority of users for the benefits of small group of Windows 7 users.

We could also use GetFileType to decide which version of the code to run. But I am bit fuzzy about this logic. For example, I printed GetFileType of stdin, stdout and stderr while running program above, and that is what I see:

X:\>type a
diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go
index 253e9e8c1f..c6c39933b6 100644
--- a/src/syscall/exec_windows.go
+++ b/src/syscall/exec_windows.go
@@ -321,6 +321,10 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
        fd := make([]Handle, len(attr.Files))
        for i := range attr.Files {
                if attr.Files[i] > 0 {
+
+       ft, _ := GetFileType(Handle(attr.Files[i]))
+       println("ft=", ft)
+
                        err := DuplicateHandle(p, Handle(attr.Files[i]), parentProcess, &fd[i], 0, true, DUPLICATE_SAME_ACCESS)
                        if err != nil {
                                return 0, 0, err

X:\>test
bug
ft= 2
ft= 2
ft= 2
2021/05/07 12:15:05 fork/exec C:\Windows\system32\cmd.exe: Insufficient system resources exist to complete the requested service.

X:\>test a
io.Copy
ft= 2
ft= 3
ft= 2
abc

X:\>

Alex

If it's too hard to solve properly, perhaps we just try CreateProcess two times. If the first one fails with ERROR_NO_SYSTEM_RESOURCES, then we create goroutines to do the copy and retry one more time?

We don't need to use goroutines here. If first CreateProcess fails with ERROR_NO_SYSTEM_RESOURCES, we could call CreateProcess second time immediately, on the same goroutine.

Alex

By using goroutine to copy, I basically mean this: ...

Sure. You can do this in your program.

Unfortunately the problem is in syscall.StartProcess. For example, some program might call syscall.StartProcess directly. We want to fix that program too.

Alex

The question is, does the issue affect only stdin/stdout/stderr or it affects all console fds inherited by the child process?

If we don't use STARTUPINFOEXW, there are no other fds exported from Go parent process to its child but stdin, stdout and stderr.

Alex

I suspect the thing to do on Windows 7 might be to actually exclude console handles from PROC_THREAD_ATTRIBUTE_HANDLE_LIST, because on Windows 7, they're not real kernel handles but rather special user mode handles with the top bits set. I need to investigate a bit, but I think this might be solvable.

https://github.com/rprichard/win32-console-docs/blob/master/README.md has some great notes and https://github.com/rprichard/win32-console-docs/blob/master/src/HandleTests/CreateProcess_InheritList.cc is instructive.

Change https://golang.org/cl/319310 mentions this issue: syscall: do not pass console handles to PROC_THREAD_ATTRIBUTE_HANDLE_LIST on Windows 7

Just posted a fix. Please try that out and let me know if it does the trick?

I'd suggest adding a test to CL 319310 though.

I'd like this too, but it's a bit hard: it would need to write to a console window, which on windows 7 is a user space concept that involves the GUI, which we probably don't want as part of tests. If you have any ideas on how to avoid UI but still test console handles, do let me know.

@ianlancetaylor is it acceptable to import x/sys/windows in a Go stdlib test?

Well, we are currently vendoring x/sys/windows under cmd, so I suppose that it wouldn't be a disaster to move that vendoring up a level. In general though I would prefer to stop vendoring x/sys/windows rather than start using it even more.

it would need to write to a console window ... which we probably don't want as part of tests.

And why we could not make https://play.golang.org/p/rzblvBHqLb2 into a test?

Alex

@alexbrainman That relies on observing that the console window has the text. Doing that programatically in Go usually involves using a pipe, but on Windows 7, a pipe handle is a kernel object, so things will work expected. The bug manifests when using a console handle, rather than a pipe handle, which is not a kernel object on Windows 7. So this means the test passes when you see the text written on a console. How do you plan to check that programatically? There's actually ReadConsoleOuput, which might work, with selecting a rectangle. I started playing with that the other day, but it seemed like a waste of time. You'd need the UI too, anyway, to even have a console (psuedo consoles are new in Windows 10), and I don't think we want UIs in our tests.

So, I have no interest in writing a test case for this. If you want to try to figure it out, go for it, but I'm not going to spend the time.

However, https://golang.org/cl/319310 fixes the issue, and I'd appreciate it if you could review that so that we can submit it.

@alexbrainman ...

I replied on https://go-review.googlesource.com/c/go/+/319310/

Alex

Change https://golang.org/cl/325112 mentions this issue: syscall: regenerate zsyscall_windows.go