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

Different behavior of exit listener of spawned process #25137

Closed
RoobinGood opened this issue Dec 19, 2018 · 2 comments
Closed

Different behavior of exit listener of spawned process #25137

RoobinGood opened this issue Dec 19, 2018 · 2 comments
Labels
child_process Issues and PRs related to the child_process subsystem. question Issues that look for answers.

Comments

@RoobinGood
Copy link

Process started by child_process module can start its own child process. For example tar can start child process if we use it in concrete directory. And if we choose unexisting directory for archive destination it can fail in different way:

$ tar -C /some/directory -cvzf /path/to/archive.tgz fileToArchive
tar (child): /path/to/archive.tgz: Cannot open: No such file or directory
tar (child): Error is not recoverable: exiting now
example.js
$ echo $?
141

# and sometimes (1 from 10 or more runs) it can receive exit code 2 and finish correctly
$ tar -C /some/directory -cvzf /path/to/archive.tgz fileToArchive
fileToArchive
tar (child): /path/to/archive.tgz: Cannot open: No such file or directory
tar (child): Error is not recoverable: exiting now
tar: Child returned status 2
tar: Error is not recoverable: exiting now
$ echo $?
2

The problem here is that on NodeJS <8.2.0 it's always exit with code 2 because child process emit this exit code.
For example this script example.js:

const spawn = require('child_process').spawn;
const Steppy = require('twostep').Steppy;
const pathUtils = require('path');
const assert = require('assert');

const exec = function(cmd, args) {
	return new Promise((resolve, reject) => {
		cmdSpawn = spawn(
			cmd,
			args,
			{stdio: ['pipe', 'pipe', 'pipe']}
		);

		let stdoutData = '';
		cmdSpawn.stdout.on('data', (data) => {
			stdoutData += data;
		});

		let stderrData = '';
		cmdSpawn.stderr.on('data', (data) => {
			stderrData += data;
		});

		cmdSpawn.on('exit', (exitCode, signal) => {
			console.log(exitCode, signal)
			resolve({
				stdoutData,
				stderrData,
				exitCode
			});
		});

		cmdSpawn.on('error', (err) => {
			promise.reject(new Error(err));
		});
	});
};


exec('tar', [
	'-C',
	__dirname,
	'-cvzf',
	pathUtils.join(__dirname, '/unexisted/path/archive.tgz'),
	'example.js'
])
	.then((result) => {
		console.log(result);

		assert.equal(result.exitCode, 2);
	})
	.catch((err) => {
		console.error(err.stack || err);
	});

will receive exit code 2 always on NodeJS 8.1.4 and this result on 8.2.0:

$ node example.js 
null 'SIGPIPE'
{ stdoutData: 'example.js\n',
  stderrData: 'tar (child): /home/robingood/work/gitlab/parking/price-calculator/scripts/unexisted/path/archive.tgz: Cannot open: No such file or directory\ntar (child): Error is not recoverable: exiting now\n',
  exitCode: null }
AssertionError [ERR_ASSERTION]: null == 2
    at exec.then (/home/robingood/work/gitlab/parking/price-calculator/scripts/example.js:50:10)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)

Why it happens? Is it correct behavior? What is right workaround for this case: handle signal in exit listener?

@Fishrock123 Fishrock123 added child_process Issues and PRs related to the child_process subsystem. question Issues that look for answers. labels Dec 19, 2018
@bnoordhuis
Copy link
Member

handle signal in exit listener?

Yes. The change in behavior you're seeing between v8.1.4 and v8.2.0 is probably due to the bug fix from libuv/libuv#1376. The relevant commit log:

unix: reset signal disposition before execve()

Signal dispositions are inherited by child processes.  Libuv itself
does not touch them (if you don't use uv_signal_start(), that is)
but the embedder might and probably does in the case of SIGPIPE.

Reset the disposition for signals 1-31 to their defaults right before
execve'ing into the new process.

@okv
Copy link

okv commented Dec 25, 2018

@bnoordhuis thanks for the detailed explanation 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

4 participants