Repository navigation
win: possible regression related to UV_PROCESS_WINDOWS_HIDE change #1625
Description
Here is a before/after of win: let UV_PROCESS_WINDOWS_HIDE hide consoles via identical commits of MoarVM/Perl6 on Win10, where we can notice a change in how an inherited handle works:
before (output of child is seen)
C:\Users\nloga\foo>perl6 -e "my $proc = Proc::Async.new('cmd.exe','/c','dir'); my $promise = $proc.start; await $promise; say 'done'; say $promise.result"
Volume in drive C has no label.
Volume Serial Number is 22C4-E412
Directory of C:\Users\nloga\foo
11/10/2017 02:40 PM <DIR> .
11/10/2017 02:40 PM <DIR> ..
0 File(s) 0 bytes
2 Dir(s) 32,038,256,640 bytes free
done
Proc.new(in => IO::Pipe, out => IO::Pipe, err => IO::Pipe, exitcode => 0, signal => 0, command => ["cmd.exe", "/c", "dir"])
after (output of child is -not- seen)
C:\Users\nloga\foo>perl6 -e "my $proc = Proc::Async.new('cmd.exe','/c','dir'); my $promise = $proc.start; await $promise; say 'done'; say $promise.result"
done
Proc.new(in => IO::Pipe, out => IO::Pipe, err => IO::Pipe, exitcode => 0, signal => 0, command => ["cmd.exe", "/c", "dir"])
It is possible MoarMV/src/io/procops.c#L629 could be doing something wrong with UV_INHERIT_FD, but I'm not sure how to determine this (and hence give you a more pure example).
This hints that additional checks may be needed somewhere:
if (sInfo.hStdInput != GetStdHandle(STD_INPUT_HANDLE) &&
sInfo.hStdOutput != GetStdHandle(STD_OUTPUT_HANDLE) &&
sInfo.hStdError != GetStdHandle(STD_ERROR_HANDLE) &&
(flags & RUN_PROCESS_IN_NEW_GROUP) == 0)
dwFlags |= CREATE_NO_WINDOW; // Run without console window only when both output and error are redirected
Activity
bnoordhuis commentedon Nov 13, 2017
@bzoz @cjihrig Ideas? I don't have any. No regressions have been reported against Node.js, to the best of my knowledge.
edit: forgot to mention, MoarVM's code looks correct to me.
cjihrig commentedon Nov 13, 2017
I'm not really sure either. Microsoft's documentation for the different flags isn't very good in this case. I don't recall any Node issues either, and IIRC, this is what Electron has been doing for a while now.
ugexe commentedon Nov 13, 2017
The flag was just added to Node (whereas it was already in use by MoarVM), so Node cannot expose a regression here can it?
bnoordhuis commentedon Nov 13, 2017
No, but we'd probably have had a report by now if the new flag caused the behavior you describe in node.
win: fix handles with UV_PROCESS_WINDOWS_HIDE
ugexe commentedon Nov 14, 2017
My current theory is electron had this problem before but later fixed it with additional logic not present in libuv:
https://github.com/electron/electron/blob/c2ab4f711f500cb6b6caa4ce587942763c9cb0d0/atom/app/atom_main.cc#L84-L86
which seems plausible given:
RouteStdioToConsole()
Similar issue leading back to RouteStdioToConsole()
ugexe commentedon Nov 18, 2017
I believe I can show a regression in nodejs:
windows (
windowsHide = true) : deadlockswindows (
windowsHide = false) : outputs directory contentsosx/linux (
windowsHide = true) : outputs directory contentswin: CREATE_NO_WINDOW when stdio is not inherited
win: CREATE_NO_WINDOW when stdio is not inherited
win: CREATE_NO_WINDOW when stdio is not inherited