Tests hang on Windows if run with PIPE stdin

[segrey]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When jest tests are run on Windows not in band and with redirected streams, the tests may hang indefinitely until force stopped.

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

Setup a test set eligible for parallel running: unspecified maxWorkers, test count > 1, tests run > 3000 ms (e.g. https://repl.it/G4F4/2).
Since test hanging is reproduced with redirected streams only and it's reproduced from time to time, there is a simple stress-testing.js script running jest tests endlessly:

const child_process = require('child_process');

function runProcess(id) {
  const child = child_process.spawn(process.execPath, [
    'node_modules\\jest\\bin\\jest.js'
  ], {
    stdio: 'pipe'
  });
  child.stdout.on('data', function (data) {
    process.stdout.write(`${id} stdout: ${data}`);
  });
  child.stderr.on('data', function (data) {
    process.stderr.write(`${id} stderr: ${data}`);
  });
  child.on('close', function (code) {
    console.log(`${id} child process exited with code ${code}`);
    if (code !== 0) {
      console.log('Abnormal termination');
    }
    else {
      runProcess(id + 1);
    }
  })
}

runProcess(1);

What is the expected behavior?
Tests don't hang.

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.
jest 19.0.2
node 7.7.3
npm 4.4.1
OS Windows 10, 64-bit

[segrey]

Looks like it's caused by accessing process.stdin property in Object.assign({}, process, toStringOverwrite). It's a known Node.js issue (nodejs/node#10836).
As a workaround, child process could be spawned with pipe stdin, like

  var child = childProcess.fork(childModule, {
          env: process.env
        , cwd: process.cwd()
        , stdio: ['pipe', 'inherit', 'inherit', 'ipc']
      })

https://github.com/rvagg/node-worker-farm/blob/v1.3.1/lib/fork.js

[segrey]

Another workaround could be to replace this line

  global.process = Object.assign({}, process, toStringOverwrite);

from https://github.com/facebook/jest/blob/v19.0.2/packages/jest-util/src/installCommonGlobals.js#L42
with

  global.process = new Proxy(process, {
    get: function(target, name) {
      if (toStringOverwrite.hasOwnProperty(name)) {
        return toStringOverwrite[name];
      }
      return target[name];
    }
  });

[thymikee]

@wtgtybhertgeghgtwtg can you weigh in?

[wtgtybhertgeghgtwtg]

Not to pass the buck, but isn't this a worker-farm issue? jest only directly touches child_process in jest-changed-files, jest-editor-support, and jest-haste-map; TestRunner defers to worker-farm.

[segrey]

Thanks for looking into it! Even if it's a Node.js issue, applying a workaround on jest side could be beneficial for jest users.
Not sure which workaround suits better. Since worker-farm is a general-purpose package, default stdio looks reasonable. Let me explain why this workaround works: stdin='pipe' forces a new stdin handle creation; with stdin='inherit', the handle is reused and some bug in concurrent (from several processes) stdin access leads to hanging on Windows.
Avoiding process.stdin access in jest-util/src/installCommonGlobals.js would fix it too (see my second workaround).

[wtgtybhertgeghgtwtg]

Apologies if I'm misunderstanding, but I don't see how the first workaround can be used without pulling worker-farm functions into jest. And it looks like the second workaround is for node >= 6.
But, yeah, it's probably gonna end up with either jest or worker-farm having to patch this.

[segrey]

@wtgtybhertgeghgtwtg You're correct. Either worker-farm functions needs to be pulled into jest or a separate issue in worker-farm project should be raised. BTW, now it seems that using stdio: ['pipe', 'inherit', 'inherit', 'ipc'] is preferable in general case, because several child processes can't read from the same stdin handle.

As for the second workaround, indeed, it won't work with node < 6. Anyway, Symbol.toStringTag also requires node >= 6 if run without --harmony. Update: it should be new Proxy({}, ...) to not invoke setters on real process object:

  global.process = new Proxy({}, {
    get: function(target, name) {
      if (toStringOverwrite.hasOwnProperty(name)) {
        return toStringOverwrite[name];
      }
      return process[name];
    }
  });

[cpojer]

We don't have Windows machines here at the moment, if you'd like to see this fixed, please send a PR with a test and we'll merge it. Thank you! From our side, this is not actionable, so I'm closing this issue.

[mdekrey]

Shouldn't this be labeled with "Help Wanted" and not closed?

[thymikee]

@mdekrey can help to test it on v21 and jest@test release? In test release we've changed underlying worker spawning logic, so this could get fixed by the way.

[mdekrey]

That looks like it did it, at least for me @thymikee! Thanks!

For anyone else who comes across this, 21.3.0-beta.13 was the specific version that worked for me.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.