Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spawn EAGAIN produces a sequence of uncaught exceptions that cannot be handled #32943

Closed
ggoodman opened this issue Apr 20, 2020 · 3 comments
Closed

Comments

@ggoodman
Copy link
Contributor

  • Version: 12.16.1
  • Platform: Linux eb68db941f4a 5.3.0-1016-aws #17~18.04.1-Ubuntu SMP Fri Mar 27 20:11:52 UTC 2020 x86_64 GNU/Linux
  • Subsystem: child_process

What steps will reproduce the bug?

Produced by code that looks like this:

    this.process = spawn(this.command, this.commandArgs, {
      env: this.env,
    });

    this.process.once('exit', this.onProcessExit);
    this.process.stdout.on('data', this.onProcessStdOut);
    this.process.stderr.on('data', this.onProcessStdErr);
  1. In an async function, we call child_process.spawn().
  2. process.on('uncaughtException') gets triggered with an EGAIN on the attempt to spawn the child process. The stack for this uncaught exception looks like [1]. Note that while the triggering condition was in an async function, it was not converted into a promise rejection as I would have expected. The fact that the following exceptions were reported after, despite having synchronous call-stacks back to our call to spawn makes me think this is happening outside JavaScript-land.
  3. Next, we get two identical uncaught exceptions, having a stack originating at our code calling spawn [2]. I suspect these are for stdout / stderr, respectively.

How often does it reproduce? Is there a required condition?

This was produced within a docker container on an ~80% loaded host with several hundred identical containers in operation.

What is the expected behavior?

EAGAIN exceptions thrown by spawn should be handle-able by user-space code and should not produce uncaught exceptions outside normal control flow.

What do you see instead?

As described above, what appears to be an un-handleable exception thrown outside normal control flow.

Additional information

Stack traces:

1:

uncaught exception Error: spawn /usr/local/bin/node EAGAIN
    at Process.ChildProcess._handle.onexit (internal/child_process.js:267:19)
    at onErrorNT (internal/child_process.js:469:16)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)

2:

uncaught exception Error: read ENOTCONN
    at tryReadStart (net.js:567:20)
    at Socket._read (net.js:578:5)
    at Socket.Readable.read (_stream_readable.js:478:10)
    at Socket.read (net.js:618:39)
    at new Socket (net.js:372:12)
    at Object.Socket (net.js:263:41)
    at createSocket (internal/child_process.js:314:14)
    at ChildProcess.spawn (internal/child_process.js:437:23)
    at spawn (child_process.js:548:9)
    at SandboxRuntime.spawn (/path/to/out/call/to/spawn.js:123:45)

I also note this comment that makes me suspect the libuv / node-core boundary: #21203 (comment)

@mscdex
Copy link
Contributor

mscdex commented Apr 20, 2020

Are you missing an 'error' event handler on this.process?

@ggoodman
Copy link
Contributor Author

ggoodman commented Apr 20, 2020

Are you missing an 'error' event handler on this.process?

Yes, we were missing an error event handler on this.process.

I understand that this would help us with an exception thrown in next tick:

process.nextTick(onErrorNT, this, err);

However, how would we avoid the 2nd uncaught exception, generated synchronously on the stdio streams? Is that 2nd error being thrown in a future tick despite what it appears to suggest via its stack trace? Might we have been OK by registering error handlers on this.process.{stdin,stdout}?

@bnoordhuis
Copy link
Member

Might we have been OK by registering error handlers on this.process.{stdin,stdout}?

Yes, that should work. That's something you'll probably want to do anyway because a child process abruptly terminating could cause the same behavior.

I'm going to close this out but if you have follow-up questions, I can move this to nodejs/help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants