Commit Graph

21 Commits

Author SHA1 Message Date
Anna Henningsen
036fbdb63d src: remove Environment::tracing_agent_writer()
As per the conversation in https://github.com/nodejs/node/issues/22513,
this is essentially global, and adding this on the Environment
is generally just confusing.

Refs: https://github.com/nodejs/node/issues/22513
Fixes: https://github.com/nodejs/node/issues/22767

PR-URL: https://github.com/nodejs/node/pull/23781
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
2018-10-24 11:18:47 -03:00
cjihrig
7ee8323725 src: reduce variable scope in node_worker.cc
PR-URL: https://github.com/nodejs/node/pull/23297
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-10-09 08:33:33 -04:00
Anna Henningsen
d527dde360 src: use JS inheritance for AsyncWrap
For all classes descending from `AsyncWrap`, use JS inheritance
instead of manually adding methods to the individual classes.

This allows cleanup of some code around transferring handles
over IPC.

PR-URL: https://github.com/nodejs/node/pull/23094
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-10-03 13:43:42 -07:00
Anna Henningsen
0434d270c1 worker: reduce MessagePort prototype to documented API
`MessagePort` is special because it has to be a C++ API
that is exposed to userland. Therefore, there is a number
of internal methods on its native prototype; this commit
reduces this set of methods to only what is documented in
the API.

PR-URL: https://github.com/nodejs/node/pull/23037
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-09-26 00:32:14 +02:00
Anna Henningsen
2d65e67305 worker: hide MessagePort init function behind symbol
This reduces unintended exposure of internals.

PR-URL: https://github.com/nodejs/node/pull/23037
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-09-26 00:32:12 +02:00
Andreas Haas
7dde560beb src: replace deprecated uses of FunctionTemplate::GetFunction
PR-URL: https://github.com/nodejs/node/pull/22993
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-09-24 05:44:28 +02:00
Anna Henningsen
e6d0ced7bb worker: only stop inspector if started
This may fix some flakiness with tests that use `worker.terminate()`.
In particular, the following failure seems like it could be related
(no consistent reproduction available, though):

```
15:30:14 not ok 187 parallel/test-heapdump-worker
15:30:14   ---
15:30:14   duration_ms: 2.499
15:30:14   severity: fail
15:30:14   exitcode: 134
15:30:14   stack: |-
15:30:14     npm[6904]: src\inspector_agent.cc:729: Assertion `(client_) != nullptr' failed.
```

From https://ci.nodejs.org/job/node-test-binary-windows/20041/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=2/console

Refs: https://github.com/nodejs/node/pull/22769

PR-URL: https://github.com/nodejs/node/pull/22927
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
2018-09-18 21:26:46 -07:00
Eugene Ostroukhov
f28c6f7eef inspector: workers debugging
Introduce a NodeTarget inspector domain modelled after ChromeDevTools
Target domain. It notifies inspector frontend attached to a main V8
isolate when workers are starting and allows passing messages to
inspectors on their isolates. All inspector functionality is enabled on
worker isolates.

PR-URL: https://github.com/nodejs/node/pull/21364
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2018-09-18 09:01:33 -07:00
Eugene Ostroukhov
ab5f789e3f inspector: enable Inspector JS API in workers
PR-URL: https://github.com/nodejs/node/pull/22769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-09-17 09:49:53 -07:00
Anna Henningsen
df7ebfab4e worker: correct (de)initialization order
- Initialize `thread_exit_async_` only once the thread has been
  started. This is done since it is only triggered from the
  thread when it is exiting.
- Move the final `uv_run` to the `Worker` destructor.
  This makes sure that it is always run, regardless of whether
  the thread is actually started or not.
- Always dispose the `Isolate` before cleaning up the libuv event
  loop. This now matches the reverse order of initialization.

Fixes: https://github.com/nodejs/node/issues/22736
PR-URL: https://github.com/nodejs/node/pull/22773
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-09-14 18:55:53 +02:00
Andreas Haas
d5e7294445 src: initialize PerIsolateData eagerly
PR-URL: https://github.com/nodejs/node/pull/21983
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2018-09-07 21:07:25 +02:00
Anna Henningsen
29a71bae40 src: refactor options parsing
This is a major refactor of our Node’s parser. See `node_options.cc`
for how it is used, and  `node_options-inl.h` for the bulk
of its implementation.

Unfortunately, the implementation has come to have some
complexity, in order to meet the following goals:

- Make it easy to *use* for defining or changing options.
- Keep it (mostly) backwards-compatible.
  - No tests were harmed as part of this commit.
- Be as consistent as possible.
  - In particular, options can now generally accept arguments
    through both `--foo=bar` notation and `--foo bar` notation.
    We were previously very inconsistent on this point.
- Separate into different levels of scope, namely
  per-process (global), per-Isolate and per-Environment
  (+ debug options).
- Allow programmatic accessibility in the future.
  - This includes a possible expansion for `--help` output.

This commit also leaves a number of `TODO` comments, mostly for
improving consistency even more (possibly with having to modify
tests), improving embedder support, as well as removing pieces of
exposed configuration variables that should never have become
part of the public API but unfortunately are at this point.

PR-URL: https://github.com/nodejs/node/pull/22392
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
2018-08-22 23:37:19 +02:00
Anna Henningsen
e3bae65583 worker: fix deadlock when calling terminate from exit handler
Just before we call the `'exit'` handlers of a Worker, we drain
the public port’s message queue to ensure proper ordering.
Previously, we held the Worker’s `mutex_` during the
exit handler call, so calling `terminate()` on the worker
could lead to a deadlock if called from one of those message
handlers.

This fixes flakiness in the `parallel/test-worker-dns-terminate` test.

PR-URL: https://github.com/nodejs/node/pull/22073
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-08-09 02:11:55 +02:00
Gabriel Charette
0f3c2c64d2 src: use modern v8::Platform worker threads APIs
Precursor to removing deprecated APIs on the v8 side @
https://chromium-review.googlesource.com/c/v8/v8/+/1045310

PR-URL: https://github.com/nodejs/node/pull/21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
2018-07-26 08:35:06 +02:00
Anna Henningsen
57e301539b src: enable more detailed memory tracking
This will enable more detailed heap snapshots based on
a newer V8 API.

This commit itself is not tied to that API and could
be backported.

PR-URL: https://github.com/nodejs/node/pull/21742
Reviewed-By: James M Snell <jasnell@gmail.com>
2018-07-13 19:53:15 +02:00
Anna Henningsen
018d6186a7 src: add native debugging code to workers
Now that we have better native debugging utilities in core,
let’s use them :)

PR-URL: https://github.com/nodejs/node/pull/21423
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>
2018-06-24 23:04:55 -07:00
James M Snell
99e6ecbb17 workers,trace_events: set thread name for workers
Set the thread name for workers in trace events. Also,
use uint64_t for thread_id_ because there's really no
reason for those to be doubles

PR-URL: https://github.com/nodejs/node/pull/21246
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
2018-06-20 16:02:36 +02:00
James M Snell
b505d2a1cf Revert "workers,trace_events: set thread name for workers"
This reverts commit a24b691c6c.

PR-URL: https://github.com/nodejs/node/pull/21363
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
2018-06-15 18:40:43 -07:00
James M Snell
a24b691c6c workers,trace_events: set thread name for workers
Set the thread name for workers in trace events. Also,
use uint64_t for thread_id_ because there's really no
reason for those to be doubles

PR-URL: https://github.com/nodejs/node/pull/21246
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
2018-06-15 09:14:19 -07:00
Anna Henningsen
aa2304b8d5 worker,src: display remaining handles if uv_loop_close fails
Right now, we crash the process if there are handles remaining
on the event loop when we exit (except for the main thread).

This does not provide a lot of information about causes, though;
in particular, we don’t show which handles are pending and
who own them.

This patch adds debug output to these cases to help with the
situation.

PR-URL: https://github.com/nodejs/node/pull/21238
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2018-06-13 12:23:36 +02:00
Anna Henningsen
0df031acad worker: initial implementation
Implement multi-threading support for most of the API.

Thanks to Stephen Belanger for reviewing this change in its
original form, to Olivia Hugger for reviewing the
documentation and some of the tests coming along with it,
and to Alexey Orlenko and Timothy Gu for reviewing other
parts of the tests.

Refs: https://github.com/ayojs/ayo/pull/110
Refs: https://github.com/ayojs/ayo/pull/114
Refs: https://github.com/ayojs/ayo/pull/117

PR-URL: https://github.com/nodejs/node/pull/20876
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
2018-06-06 19:43:52 +02:00