Surprising I/O behaviour on Windows

[cpitclaudel]

Hi node.js folks,

I'd like to get your input on a problem that was first raised by Flycheck users on Windows, and escalated into an Emacs bug report. I originally though the problem was with Emacs, but the observed behaviour is specific to node.js.

Some context: Flycheck is a real-time linting package for Emacs; you give it instructions on how to interpret the output of your favourite linter, and it will run it in the background and highlight errors. Many JavaScript linters are programmed using node.js, so Flycheck ends up calling node.

A few weeks ago we started getting reports that Flycheck was causing Emacs to freeze on Windows. This was shortly after Flycheck started using pipes for linting (that is, it now feeds file contents to checkers via stdin, when possible). Upon further investigation it became clear that the issue was with Emacs hanging as it tried to send input to node.js; Flycheck would call Emacs Lisp's process-send-region with a whole buffer worth of content, and Emacs would just hang. We also noticed that sending input in small batches (less than 4kiB didn't cause the issue.

We notified the Emacs folks, who came up with a fix, but the fix is essentially node.js-specific: Emacs now makes sure to never write more to a pipe than its buffer size. Eli Zaretskii, the author of that patch, explained it as follows:

I'm not entirely sure what caused the issue.  From the Emacs side, it
looked like the other end of the pipe wasn't read at all: each time
Emacs called _write, it got the return value of -1 with errno set to
ENOSPC, which means the pipe's buffer is full.  No other program with
which I tried the same test case did this.  With those other programs,
after a few first attempts which returned -1, _write started to return
positive values, meaning that it succeeded to write some part of the
stuff.

I tried to google for similar problems related to node.js, but didn't
find anything that looked useful.

So I just went with the only workaround you reported: never write to
the pipe more than its buffer size.  I.e., subdivide the write into
chinks the size of the pipe's buffer.  This required a system call, to
find out what that size was, and it also required a small change in
the logic of the loop that writes the stuff in chunks, because it
wasn't ready for the situation that a chunk succeeds, but the very
next one returns -1 and ENOSPC -- again something that only node.js
causes somehow.

(...)

If we want to try to understand this deeper, we should be talking to
some expert on how node.js works on Windows wrt I/O on its standard
input and standard output.  Filing a bug report with node.js might be
useful, but again, only if someone there will want to talk to us so
that we could understand the problem.

If you want to file a node.js bug report, you can start by saying that
it reads its stdin very slowly: once it reads a 4K chunk from the
pipe, it takes about 40 to 60 msec before it is ready to read the next
chunk.  (I can tell because when Emacs receives ENOSPC from the pipe,
it loops retrying, waiting 20 msec between retries, and I see 2 to 3
retries, sometimes even 4, before the next successful write.)  I don't
know why it takes so long -- this was on a Core i7 system which was
otherwise very quiet.  Maybe if we understand what takes so long, we
will also understand what did Emacs do wrongly, and how to avoid that.

We're mostly satisfied with the fix on the Emacs side, but there seem to be something strange going on on Node.js' side; is there some interest on your side to try to dig deeper into this issue?

The Emacs discussion is at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22344; it took a few messages to converge to the understanding presented above, though.

[mscdex]

/cc @nodejs/platform-windows

[orangemocha]

Thanks for reporting this issue, @cpitclaudel . If you were able to provide a simple test case that exposes this issue, that would be helpful. In any case, I'll put this on my queue of issues to investigate further.

[cpitclaudel]

@orangemocha It's very tricky: I don't use Windows in general, and I have no experience with Node.

However, if you use Emacs, then this is the smallest I could get on node.js' side:

• In a file C:\blackhole.js put this:

function blackhole() {
    var stream = process.openStdin();
    stream.setEncoding("utf-8");
    stream.on('data', function (chunk) { process.stdout.write(chunk); });
}
blackhole();

• Then from Emacs, run the following (M-:)

;; This hangs on Windows (works fine on Linux)

(let ((process  (start-process "bug" nil "node" "C:/blackhole.js")))
  (process-send-string process (make-string 4097 ?a)))

;; This works fine on both platforms:

(let ((process  (start-process "bug" nil "node" "C:/blackhole.js")))
  (dotimes (_ 25)
    (process-send-string process (make-string 4096 ?a))))

[cpitclaudel]

Feel free to ask for info on the Emacs bug thread, too. Sorry I can't help much more.

[helio-frota]

hi, this was solved/fixed ?

[Trott]

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

Maybe someone from @nodejs/platform-windows can confirm the bug and wants to work on this?

[cpitclaudel]

I don't care much either way: I don't use Windows much. But this issue seems significant, and relatively complex — it's not clear just ignoring it is the right way to proceed.

[refack]

I don't care much either way: I don't use Windows much. But this issue seems significant, and relatively complex — it's not clear just ignoring it is the right way to proceed.

@cpitclaudel I'm looking at this but the backlog is long and the day is short....

[seishun]

The reason no one has looked into it is probably because the testcase depends on Emacs. If you had provided a testcase that's reproducible without third-party software then more people would be interested in looking into it.

[cpitclaudel]

@seishun Yes, of course (this was pointed out earlier in the thread, too). Unfortunately, I use neither node.js nor Windows, so there's little I can do beyond reporting the issue and pointing to more competent people, as I've done :)

[baerrach]

Can I get this re-opened please?

Steps to reproduce:

• git clone https://github.com/nodejs/node.git checkout the node git repository
• .\vcbuild without-intl (only needed to seed tools\node_modules)

Failing

• type lib\fs.js | node .\tools\node_modules\eslint\bin\eslint.js --stdin

Error message:

<text>
  144:4  error  Parsing error: Unexpected token

  142 | function maybeCallback(cb) {
  143 |   if (typeof cb === 'function')
> 144 |
      |    ^

✖ 1 problem (1 error, 0 warnings)

The process tried to write to a nonexistent pipe.

Succeeding

• node .\tools\node_modules\eslint\bin\eslint.js lib\fs.js

[baerrach]

Interestingly when running under Git for Windows Bash it works fine.

cat lib/fs.js | node ./tools/node_modules/eslint/bin/eslint.js --stdin

(make sure you dont have an alias for node that prefixes with winpty or else you will get stdin is not a tty issues)

[baerrach]

@cpitclaudel I just found https://github.com/nodejs/node/blob/master/doc/api/process.md#a-note-on-process-io where it states Pipes (and sockets): synchronous on Windows, this is why Emacs needs to send the buffer in chunks.

And the process.stdin.isTTY examples made me think that piping the output might be causing issues, why not try redirection as well?

Succeeding

• CMD: node .\tools\node_modules\eslint\bin\eslint.js --stdin < lib\fs.js
• Bash for Window: node ./tools/node_modules/eslint/bin/eslint.js --stdin < lib/fs.js

I'm trying to track down a fix for the emacs/flycheck problem and it uses pipes to communicate to the spawned eslint process, stdin redirection will not work.

[baerrach]

Okay, I've tracked this down to eslint's problem.

https://github.com/eslint/eslint/blob/master/bin/eslint.js#L63

    /*
     * Note: `process.stdin.fd` is not used here due to https://github.com/nodejs/node/issues/7439.
     * Accessing the `process.stdin` property seems to modify the behavior of file descriptor 0, resulting
     * in an error when stdin is piped in asynchronously.
     */
    const STDIN_FILE_DESCRIPTOR = 0;

    process.exitCode = cli.execute(process.argv, fs.readFileSync(STDIN_FILE_DESCRIPTOR, "utf8"));

But this fails because of the 4k buffer size limit.

Hacking their code in my environment to use a copy-pasta of https://github.com/nodejs/node/blob/master/doc/api/process.md#processstdin

  const input = [];

  process.stdin.on('readable', () => {
    let chunk;
    // Use a loop to make sure we read all available data.
    while ((chunk = process.stdin.read()) !== null) {
      input.push(chunk);
    }
  });

  process.stdin.on('end', () => {
    const contents = input.join('');
    process.exitCode = cli.execute(process.argv, contents, "utf8");
  });

And it works.

Can I get someone who has a clue about this give me some direction?

Should I submit a PR to eslint with the above code?

[Trott]

@not-an-aardvark

[not-an-aardvark]

Feel free to submit a PR. It seems like the current code there had some workarounds to avoid an unrelated issue with reading stdin on macOS, so we would need to make sure the new code works cross-platform. (As an aside, it would be great if Node had a reliable way to read all of stdin synchronously and/or without needing 14 lines of code.)

[jasnell]

There's been no activity on this and it's not clear how/when it will progress. Keeping the issue open does not benefit anyone but we also don't want to forget about it. I've added it to a Futures project board so it doesn't get lost.