Commit Graph

13 Commits

Author SHA1 Message Date
Fedor Indutny
6712a1faa5 internal/child_process: call postSend on error
Call `obj.postSend` in error case of `process.send()`. The
`net.Socket`'s handle should not be leaked.

Note that there are two callbacks invoked on handles
when they are sent to the child process over IPC pipes.
These callbacks are specified by `handleConversion` object, and
during send two of them are invoked:

  * `send`
  * `postSend`

Now for `net.Socket` in particular, `postSend` performs clean up by
closing the actual uv handle. However this clean up will not happen in
one of the branches. This pull request aims to fix this.

PR-URL: https://github.com/nodejs/node/pull/4752
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
2016-01-22 13:27:18 -05:00
Michaël Zasso
7ce0e04f44 lib: fix style issues after eslint update
With an indentation style of two spaces, it is not possible to indent
multiline variable declarations by four spaces. Instead, the var keyword
is used on every new line.
Use const instead of var where applicable for changed lines.

PR-URL: https://github.com/nodejs/io.js/pull/2286
Reviewed-By: Roman Reiss <me@silverwind.io>
2016-01-13 23:16:05 +01:00
Rich Trott
57a51a00e8 child_process: guard against race condition
It is possible that the internal hnadleMessage() might try to send to
a channel that has been closed. The result can be an AssertionError.
Guard against this.

Fixes: https://github.com/nodejs/node/issues/4205
PR-URL: https://github.com/nodejs/node/pull/4418
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
2015-12-27 13:10:28 -08:00
Dave
34b535f4ca child_process: flush consuming streams
When a client calls read() with a nonzero argument
on a Socket, that Socket sets this._consuming to true.
It never sets this._consuming back to false.
ChildProcess.flushStdio() currently doesn't flush
any streams where _consuming is truthy. But, that means
that it never flushes any stream that has ever been read from.
This prevents a child process from ever closing if one of
its streams has been read from, causing issue #4049. This
commit allows consuming streams to be flushed, and the
child process to emit a close event.

Fixes: https://github.com/nodejs/node/issues/4049
PR-URL: https://github.com/nodejs/node/pull/4071
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2015-12-01 09:55:35 -05:00
micnic
20285ad177 lib: Consistent error messages in all modules
This commit fixes some error messages that are not consistent with
some general rules which most of the error messages follow.

PR-URL: https://github.com/nodejs/node/pull/3374
Reviewed-By: Roman Reiss <me@silverwind.io>
2015-11-09 20:08:36 +01:00
Rich Trott
cf0130dc0d lib: return boolean from child.send()
Previous change reinstated returning boolean from child.send() but
missed one instance where undefined might be returned instead.

PR-URL: https://github.com/nodejs/node/pull/3577
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
2015-10-29 17:00:27 -07:00
Rich Trott
cdcf00a0b9 lib,doc: return boolean from child.send()
The documentation indicates that child.send() returns a boolean but it
has returned undefinined at since v0.12.0. It now returns a boolean per
the (slightly updated) documentation.

PR-URL: https://github.com/nodejs/node/pull/3516
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
2015-10-28 16:21:36 -07:00
Fedor Indutny
36b969ff44 child_process: null channel handle on close
`HandleWrap::OnClose` destroys the underlying C++ object and null's the
internal field pointer to it. Therefore there should be no references to
the wrapping JavaScript object.

`null` the process' `_channel` field right after closing it, to ensure
no crashes will happen.

Fix: https://github.com/nodejs/node/issues/2847
PR-URL: https://github.com/nodejs/node/pull/3041
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
2015-09-25 00:17:45 -04:00
Sakthipriyan Vairamani
f32a606e37 lib,src: remove usage of events.EventEmitter
The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.

Refer: https://github.com/nodejs/node/pull/2896

PR-URL: https://github.com/nodejs/node/pull/2921
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
2015-09-23 00:23:08 +05:30
Evan Lucas
3bc7e58ab0 child_process: use stdio.fd even if it is 0
Previously, in _validateStdio we were using stdio.fd || stdio. If
stdio.fd was falsy (or 0 in the case of stdin), then the entire stdio
object would be passed which could cause a crash.

Fixes: https://github.com/nodejs/node/issues/2721
PR-URL: https://github.com/nodejs/node/pull/2727
Reviewed-By: silverwind - Roman Reiss <me@silverwind.io>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>
2015-09-07 19:03:19 -05:00
Ben Noordhuis
607aa3ac82 child_process: add callback parameter to .send()
Add an optional callback parameter to `ChildProcess.prototype.send()`
that is invoked when the message has been sent.

Juggle the control channel's reference count so that in-flight messages
keep the event loop (and therefore the process) alive until they have
been sent.

`ChildProcess.prototype.send()` and `process.send()` used to operate
synchronously but became asynchronous in commit libuv/libuv@393c1c5
("unix: set non-block mode in uv_{pipe,tcp,udp}_open"), which landed
in io.js in commit 07bd05b ("deps: update libuv to 1.2.1").

Fixes: https://github.com/nodejs/node/issues/760
PR-URL: https://github.com/nodejs/node/pull/2620
Reviewed-By: trevnorris - Trevor Norris <trev.norris@gmail.com>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
2015-09-06 21:37:58 +10:00
Roman Reiss
b5b8ff117c lib: don't use global Buffer
Port of https://github.com/joyent/node/pull/8603

The race condition present in the original PR didn't occur, so no
workaround was needed.

PR-URL: https://github.com/nodejs/io.js/pull/1794
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
2015-06-11 20:24:44 +02:00
Evan Lucas
a77c330c32 child_process: expose ChildProcess constructor
Creates two new internal modules (child_process and socket_list) for
better readability.

Exposes the ChildProcess constructor from the child_process module so
one can now `require(‘child_process’).ChildProcess`

Fixes: https://github.com/nodejs/io.js/issues/1751
PR-URL: https://github.com/nodejs/io.js/pull/1760
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
2015-05-28 09:38:29 -05:00