On UNIX platforms, the debugger doesn't reliably kill the inferior when
killed by a signal. Work around that by spawning the debugger in its
own process group and killing the process group instead of just the
debugger process.
This is a hack to get the continuous integration back to green, it
doesn't address the underlying issue, which is that the debugger
shouldn't leave stray processes behind.
Fixes: https://github.com/nodejs/node/issues/7034
PR-URL: https://github.com/nodejs/node/pull/7037
Refs: https://github.com/nodejs/node/pull/3470
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit fix a possible crash situation in dgram send().
A crash is possible if an array is passed, and then altered after the
send call, as the call to libuv is wrapped in process.nextTick().
It also avoid sending an empty array to libuv by allocating an empty
buffer. It also does some cleanup inside send() to increase readability.
It removes test flakyness by use common.mustCall and
common.platformTimeout. Fixes situations were some events were not
asserted to be emitted.
Fixes: https://github.com/nodejs/node/issues/6616
PR-URL: https://github.com/nodejs/node/pull/6804
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.
This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.
PR-URL: https://github.com/nodejs/node/pull/6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
cluster's internal message handling includes a cache of callback
functions. Once the message for that callback is received, it is
removed from the cache. If, for any reason, the same message ID
is processed twice, the callback will be missing from the cache
and cluster will try to call undefined as a function. This commit
guards against this scenario.
Refs: https://github.com/nodejs/node/issues/6561
PR-URL: https://github.com/nodejs/node/pull/6902
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
v8::Object::GetAlignedPointerFromInternalField() returns a random value
if Wrap() hasn't been run on the object handle. Causing v8 to abort if
certain getters are accessed. It's possible to access these getters and
functions during class construction through the AsyncWrap init()
callback, and also possible in a subset of those scenarios while running
the persistent handle visitor.
Mitigate this issue by manually setting the internal aligned pointer
field to nullptr in the BaseObject constructor and add necessary logic
to return appropriate values when nullptr is encountered.
PR-URL: https://github.com/nodejs/node/pull/6184
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Though the TLSWrap constructor is only called via TLSWrap::Wrap() (i.e.
tls_wrap.wrap()) internally, it is still exposed to JS. Don't allow the
application to abort by inspecting the instance before it has been
wrap'd by another handle.
PR-URL: https://github.com/nodejs/node/pull/6184
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Don't cache the exported values of fully uninitialized builtins.
This works by adding an additional `loading` flag that is only
active during initial loading of an internal module and checking
that either the module is fully loaded or is in that state before
using its cached value.
This has the effect that builtins modules which could not be loaded
(e.g. because compilation failed due to missing stack space) can be
loaded at a later point.
Fixes: https://github.com/nodejs/node/issues/6899
PR-URL: https://github.com/nodejs/node/pull/6907
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
As it is, `fs.mkdtemp` crashes with a C++ assertion if the callback
function is not passed. This patch uses `maybeCallback` to create one,
if no callback function is passed.
PR-URL: https://github.com/nodejs/node/pull/6828
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
The only tests for `addMembership()` and `dropMembership()` (from the
`dgram` module) were in `test/internet` which means they almost never
get run. This adds checks in `test/parallel`.
PR-URL: https://github.com/nodejs/node/pull/6753
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Previously a `checks.workers` boolean was conditionally set, but never
checked. Additionally, it was never actually set because
`cluster.onlineWorkers` is always undefined.
PR-URL: https://github.com/nodejs/node/pull/6535
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This is purely cleanup and carries no visible behavioural changes.
Up to now, `this._closed` was used in zlib.js as a
synonym of `!this._handle`. This change makes this connection
explicit and removes the `_closed` property from zlib streams,
as the previous duplication has been the cause of subtle errors
like https://github.com/nodejs/node/issues/6034.
This also makes zlib errors lead to an explicit `_close()` call
rather than waiting for garbage collection to clean up the handle,
thus returning memory resources earlier in the case of an error.
Add a getter for `_closed` so that the property remains accessible
by legacy code.
PR-URL: https://github.com/nodejs/node/pull/6574
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The require('constants') module is currently undocumented and mashes
together unrelated constants. This refactors the require('constants')
in favor of distinct os.constants, fs.constants, and crypto.constants
that are specific to the modules for which they are relevant. The
next step is to document those within the specific modules.
PR-URL: https://github.com/nodejs/node/pull/6534
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com>
The only tests for `setBroadcast()` (from the `dgram` module) were in
`test/internet` which means they almost never get run. This adds a
minimal test that can check JS-land functionality in `test/parallel`.
I also expanded a comment and did some minor formatting on the existing
`test/internet` test. If there were an easy and reliable way to check
for the BROADCAST flag on an interface, it's possible that a version of
the test could be moved to `test/sequential` or `test/parallel` once it
was modified to only use internal networks.
PR-URL: https://github.com/nodejs/node/pull/6750
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The custom linting rule for argument alignment in multi-line function
calls previously ignored template strings in an effort to avoid false
positives. This isn't really necessary. Enforce for template strings and
adjust whitespace in three tests to abide. (Insert "The test abides"
joke of your choosing here.)
PR-URL: https://github.com/nodejs/node/pull/6720
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Adjust style in doctool tests to conform with predominant style of the
rest of the project. The biggest changes are:
* Replace string concatenation with `path.join()`
* Remove unnecessary quotes from property names
PR-URL: https://github.com/nodejs/node/pull/6719
Reviewed-By: James M Snell <jasnell@gmail.com>
Replace lightly-used services file parsing in favor of
confirming one of a small number of allowable values in service name
lookup tests.
In https://github.com/nodejs/node-v0.x-archive/issues/8047, it was
decided that this sort of service file parsing was superior to
hardcoding acceptable values, but I'm not convinced:
* No guarantee that the host uses /etc/services before, e.g., nscd.
* Increases complexity of tests without guaranteeing robustness.
I think that simply checking against a small set of expected values
may be a better solution. Ideally, there would also be a unit test that
used a test double for the appropriate `cares` function and confirms
that it is called with the correct parameters, but now we're getting way
ahead of ourselves.
PR-URL: https://github.com/nodejs/node/pull/6709
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Until now, the docs stated that `process.noDeprecation` could be set
at runtime, but before any modules were loaded. That was not true,
because `lib/internal/util.js` was loaded during the process startup
process, so setting the flag at runtime was pointless.
Minimal test case:
process.noDeprecation = true;
process.EventEmitter;
This patch moves checking `process.noDeprecation` to the place where
it was actually used.
PR-URL: https://github.com/nodejs/node/pull/6683
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add the `--preserve-symlinks` flag. This makes the changes added
in #5950 conditional. By default the old behavior is used. With
the flag set, symlinks are preserved, switching to the new
behavior. This should be considered to be a temporary solution
until we figure out how to solve the symlinked peer dependency
problem in a more general way that does not break everything
else.
Additional test cases are included.
PR-URL: https://github.com/nodejs/node/pull/6537
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Ben Noordhuis