Make windows stdio pipes async (?)

[Fishrock123]

I had been meaning to look into doing this for some time, as noted in #25638 (review).

tl;dr the only reason (so far as I could tell) that windows pipes are sync is that at the time they were made sync, unix was also sync (for some reason that I don't recall).

(They were made blocking in 20176a9 as a response to nodejs/node-v0.x-archive#3584.)

---

Anyways I tried to do it with the following patch, but it caused more CI errors than I had expected, across all windows machines, and I'm not sure I am invested enough to deal with them so I'll leave them here in case anyone is interested.

(Patch to make windows pipes async again.)

diff --git a/lib/net.js b/lib/net.js
index 9eb7448c59..6c70cae264 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -283,23 +283,6 @@ function Socket(options) {
       throw errnoException(err, 'open');
 
     this[async_id_symbol] = this._handle.getAsyncId();
-
-    if ((fd === 1 || fd === 2) &&
-        (this._handle instanceof Pipe) &&
-        process.platform === 'win32') {
-      // Make stdout and stderr blocking on Windows
-      err = this._handle.setBlocking(true);
-      if (err)
-        throw errnoException(err, 'setBlocking');
-
-      this._writev = null;
-      this._write = makeSyncWrite(fd);
-      // makeSyncWrite adjusts this value like the original handle would, so
-      // we need to let it do that by turning it into a writable, own property.
-      Object.defineProperty(this._handle, 'bytesWritten', {
-        value: 0, writable: true
-      });
-    }
   }
 
   // shut down the socket when we're finished with it.

...

46 Failures:

``` test.parallel/test-child-process-stdout-flush-exit test.parallel/test-cli-eval test.parallel/test-module-main-fail test.parallel/test-preload test.parallel/test-module-main-fail test.parallel/test-preload test.parallel/test-worker-memory test.parallel/test-debug-usage test.parallel/test-module-main-preserve-symlinks-fail test.parallel/test-stdio-pipe-redirect test.parallel/test-stdout-close-catch test.sequential/test-stream2-stderr-sync test.parallel/test-child-process-stdout-flush-exit test.parallel/test-debug-usage test.parallel/test-module-main-preserve-symlinks-fail test.parallel/test-stdio-pipe-redirect test.sequential/test-stream2-stderr-sync test.parallel/test-module-main-fail test.parallel/test-preload test.sequential/test-repl-timeout-throw test.parallel/test-module-main-fail test.parallel/test-preload test.sequential/test-repl-timeout-throw test.parallel/test-debugger-pid test.parallel/test-child-process-stdout-flush-exit test.parallel/test-cli-eval test.parallel/test-debug-usage test.parallel/test-module-main-preserve-symlinks-fail test.sequential/test-stream2-stderr-sync test.parallel/test-child-process-stdout-flush-exit test.parallel/test-cli-eval test.parallel/test-debug-usage test.parallel/test-module-main-preserve-symlinks-fail test.parallel/test-stdio-pipe-redirect test.parallel/test-stdout-close-catch test.sequential/test-stream2-stderr-sync test.parallel/test-child-process-stdout-flush-exit test.parallel/test-cli-eval test.parallel/test-module-main-fail test.parallel/test-preload test.sequential/test-repl-timeout-throw test.parallel/test-debug-usage test.parallel/test-module-main-preserve-symlinks-fail test.parallel/test-stdio-pipe-redirect test.parallel/test-stdout-close-catch test.sequential/test-stream2-stderr-sync ```

...

Link to the CI run while it lasts: https://ci.nodejs.org/job/node-test-commit-windows-fanned/24192/

It's possible there was some other reason that windows pipes were made sync or some other implication or side effect I don't know about, but I'm not invested enough to debug it.

Anyways I made this issue if nothing else for posterity in case people are interested.

[bzoz]

cc @nodejs/platform-windows

[jasnell]

There's been no further activity on this in over a year and no clear resolution for it. Closing but adding it to the Futures project board so it does not get lost

[addaleax]

Fwiw, this is something that should ideally still happen at some point.

[jasnell]

Perhaps, then, we should reopen with a feature request label?

[addaleax]

It’s not really a public feature. I think we could also put it under the general “stdio weirdness” umbrella, I think there’s a tracking issue for that somewhere.

[gireeshpunathil]

pls see #20000 (comment) . I think we could have a general discussion around stdio weirdness and seek consensus around what to do with it.

[CMCDragonkai]

Question, if node is setting windows pipes to blocking by default, this should mean that these 2 lines:

process.stdout._handle.setBlocking(true);
process.stderr._handle.setBlocking(true);

If added to the top of a script should have no effect right?

However this doesn't appear to be case. I have a jest test that succeeds when I set the 2 lines above, but fails when I don't have those 2 lines. These tests run on a windows CI/CD machine. Linux and MacOS are not affected.

I've done these tests repetitively toggling between true and false. But I always see an effect with those 2 lines enabled.