I noticed that we were taking `TimerCb` as a `const&` and then copying
that into the member. This is completely fine when the constructor is
called with an lvalue. However, when called with an rvalue, we can allow
the `std::function` to be moved into the member instead of falling back
to a copy, so I changed the constructors to take in universal
references. Also, `std::function` constructors can take in multiple
arguments, so I further modified the constructors to use variadic
templates.
Signed-off-by: Darshan Sen <darshan.sen@postman.com>
PR-URL: https://github.com/nodejs/node/pull/40665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Split `Stop(true)` and `Stop(false)` into separate methods since the
actions performed by these are fully distinct.
PR-URL: https://github.com/nodejs/node/pull/34252
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
There’s no point in having an opaque user data pointer when we’re
already using `std::function`.
PR-URL: https://github.com/nodejs/node/pull/34252
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Consolidate uv_timer_t boilerplate code into a shared utility.
There are several places throughout the code where we use uv_timer_t
internally (inspector, perf, quic), with some code duplication.
This eliminates the duplicated code, ensures that cleanup occurs
correctly, and simplifies use of the timers.
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/34186
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Unused since the excellent refactoring in 2930bd,
which also removed `src/timer_wrap.cc` from `node.gyp`. If you try and
get at the binding on a v11.0-pre build, you'll get an error, since the
file is no longer in the GYP build.
```
Jonathans-MBP:node jon$ ./node -v
v11.0.0-pre
Jonathans-MBP:node jon$ ./node -e "process.binding('timer_wrap')"
internal/bootstrap/loaders.js:81
mod = bindingObj[module] = getBinding(module);
^
Error: No such module: timer_wrap
at process.binding (internal/bootstrap/loaders.js:81:36)
at [eval]:1:9
at Script.runInThisContext (vm.js:89:20)
at Object.runInThisContext (vm.js:286:38)
at Object.<anonymous> ([eval]-wrapper:6:22)
at Module._compile (internal/modules/cjs/loader.js:689:30)
at evalScript (internal/bootstrap/node.js:562:27)
at startup (internal/bootstrap/node.js:248:9)
at bootstrapNodeJSCore (internal/bootstrap/node.js:595:3)
```
PR-URL: https://github.com/nodejs/node/pull/21777
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Refs: https://github.com/nodejs/node/pull/20894
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>
Extracts common setters to a single location
PR-URL: https://github.com/nodejs/node/pull/21769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Otherwise, this turns into an infinite loop when the flag
is set, because it makes `MakeCallback()` return immediately.
PR-URL: https://github.com/nodejs/node/pull/20884
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Hang all timer lists off a single TimerWrap and use the PriorityQueue
to manage expiration priorities. This makes the Timers code clearer,
consumes significantly less resources and improves performance.
PR-URL: https://github.com/nodejs/node/pull/20555
Fixes: https://github.com/nodejs/node/issues/16105
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
- Instead of storing a pointer whose type refers to the specific
subclass of `BaseObject`, just store a `BaseObject*` directly.
This means in particular that one can cast to classes along
the way of the inheritance chain without issues, and that
`BaseObject*` no longer needs to be the first superclass
in the case of multiple inheritance.
In particular, this renders hack-y solutions to this problem (like
ddc19be6de) obsolete and addresses
a `TODO` comment of mine.
- Move wrapping/unwrapping methods to the `BaseObject` class.
We use these almost exclusively for `BaseObject`s, and I hope
that this gives a better idea of how (and for what) these are used
in our code.
- Perform initialization/deinitialization of the internal field
in the `BaseObject*` constructor/destructor. This makes the code
a bit more obviously correct, avoids explicit calls for this
in subclass constructors, and in particular allows us to avoid
crash situations when we previously called `ClearWrap()`
during GC.
This also means that we enforce that the object passed to the
`BaseObject` constructor needs to have an internal field.
This is the only reason for the test change.
- Change the signature of `MakeWeak()` to not require a pointer
argument. Previously, this would always have been the same
as `this`, and no other value made sense. Also, the parameter
was something that I personally found somewhat confusing
when becoming familiar with Node’s code.
- Add a `TODO` comment that motivates switching to real inheritance
for the JS types we expose from the native side. This patch
brings us a lot closer to being able to do that.
- Some less significant drive-by cleanup.
Since we *effectively* already store the `BaseObject*` pointer
anyway since ddc19be6de, I do not
think that this is going to have any impact on diagnostic tooling.
Fixes: https://github.com/nodejs/node/issues/18897
PR-URL: https://github.com/nodejs/node/pull/20455
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Instead of using kOnTimeout index to track a special list
processing function, just pass in a function to C++ at
startup that executes all handles and determines which
function to call.
This change improves the performance of unpooled timeouts
by roughly 20%, as well as makes the unref/ref processing
easier to follow.
PR-URL: https://github.com/nodejs/node/pull/18582
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
PR-URL: https://github.com/nodejs/node/pull/18562
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Pass in Timer.now() as an argument of kOnTimeout instead of always
re-entering C++ to get it. Also don't constantly call Timer.now()
from ontimeout, even when it isn't needed. Improves performance
on our pooled benchmark by upwards of 40%.
PR-URL: https://github.com/nodejs/node/pull/18486
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Instead of using nextTick to process failed lists, just attempt to
process them again from C++ if the process is still alive.
This also allows the removal of domain specific code in timers.
The current behaviour is not quite ideal as it means that all lists
after the failed one will process on an arbitrary nextTick, even if
they're — say — not due to fire for another 2 days...
PR-URL: https://github.com/nodejs/node/pull/18486
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Refactor Immediates handling to allow for them to be unrefed, similar
to setTimeout, but without extra handles.
Document the new `immediate.ref()` and `immediate.unref()` methods.
Add SetImmediateUnref on the C++ side.
PR-URL: https://github.com/nodejs/node/pull/18139
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If an error is encountered during the processing of Immediates, schedule
the remaining queue to finish after all error handling code runs (if the
process is still alive to do so). The new changes make the Immediates
error handling behaviour entirely deterministic and predictable, as the
full queue will be flushed on each Immediates cycle, regardless of
whether an error is encountered or not.
Currently this processing is scheduled for nextTick which can yield
unpredictable results as the nextTick might happen as early as close
callbacks phase or as late as after the next event loop turns Immediates
all fully processed. The latter can result in two full cycles of
Immediates processing during one even loop turn.
The current implementation also doesn't differentiate between Immediates
scheduled for the current queue run or the next one, so Immediates that
were scheduled for the next turn of the event loop, will process
alongside the ones that were scheduled for the current turn.
PR-URL: https://github.com/nodejs/node/pull/17879
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make setImmediate() immune to `process` global tampering by removing
the dependency on the `process._immediateCallback` property.
PR-URL: https://github.com/nodejs/node/pull/17736
Fixes: https://github.com/nodejs/node/issues/17681
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit renames async-wrap to async_wrap for consitency with other
c++ source files.
PR-URL: https://github.com/nodejs/node/pull/17022
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Previously, built-in modules are registered before main() via
__attribute__((constructor)) mechanism in GCC and similiar
mechanism in MSVC. This causes some issues when node is built as
static library. Calling module registration function for built-in
modules in node::Init() helps to avoid the issues.
Signed-off-by: Yihong Wang <yh.wang@ibm.com>
PR-URL: https://github.com/nodejs/node/pull/16565
Refs: https://github.com/nodejs/node/pull/14986#issuecomment-332758206
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Adds `AsyncWrap::AddWrapMethods()` to add common methods
to a `Local<FunctionTemplate>`. Follows same pattern as
stream base.
PR-URL: https://github.com/nodejs/node/pull/14937
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Allow handles to retrieve their own uid's by adding a new method on the
FunctionTemplates. Implementation of these into all other classes will
come in a future commit.
Add the method AsyncWrap::GetAsyncId() to all inheriting class objects
so the uid of the handle can be retrieved from JS.
In all applicable locations, run ClearWrap() on the object holding the
pointer so that it never points to invalid memory and make sure Wrap()
is always run so the class pointer is correctly attached to the object
and can be retrieved so GetAsyncId() can be run.
In many places a class instance was not removing its own pointer from
object() in the destructor. This left an invalid pointer in the JS
object that could cause the application to segfault under certain
conditions.
Remove ClearWrap() from ReqWrap for continuity. The ReqWrap constructor
was not the one to call Wrap(), so it shouldn't be the one to call
ClearWrap().
Wrap() has been added to all constructors that inherit from AsyncWrap.
Normally it's the child most class. Except in the case of HandleWrap.
Which must be the constructor that runs Wrap() because the class pointer
is retrieved for certain calls and because other child classes have
multiple inheritance to pointer to the HandleWrap needs to be stored.
ClearWrap() has been placed in all FunctionTemplate constructors so that
no random values are returned when running getAsyncId(). ClearWrap() has
also been placed in all class destructors, except in those that use
MakeWeak() because the destructor will run during GC. Making the
object() inaccessible.
It could be simplified to where AsyncWrap sets the internal pointer,
then if an inheriting class needs one of it's own it could set it again.
But the inverse would need to be true also, where AsyncWrap then also
runs ClearWeak. Unforunately because some of the handles are cleaned up
during GC that's impossible. Also in the case of ReqWrap it runs Reset()
in the destructor, making the object() inaccessible. Meaning,
ClearWrap() must be run by the class that runs Wrap(). There's currently
no generalized way of taking care of this across all instances of
AsyncWrap.
I'd prefer that there be checks in there for these things, but haven't
found a way to place them that wouldn't be just as unreliable.
Add test that checks all resources that can run getAsyncId(). Would like
a way to enforce that any new classes that can also run getAsyncId() are
tested, but don't have one.
PR-URL: https://github.com/nodejs/node/pull/12892
Ref: https://github.com/nodejs/node/pull/11883
Ref: https://github.com/nodejs/node/pull/8531
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
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: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Use `static` definitions and anonymous namespaces to reduce the
number of symbols that are exported from the `node` binary.
PR-URL: https://github.com/nodejs/node/pull/12366
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The `repeat` param in `start(timeout, repeat)` was 0 in all callsites.
PR-URL: https://github.com/nodejs/node/pull/7994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/7990
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
This fixes my perceived usability issues with 7d8882b. Which, at the
time of writing, has not landed in any release except v6 RCs. This
should not be considered a breaking change due to that.
It is useful if you have a handle, even if it has been closed, to be
able to inspect whether that handle was unrefed or not. As such, this
renames the method accordingly. If people need to check a handle's
aliveness, that is a separate API we should consider exposing.
Refs: https://github.com/nodejs/node/pull/5834
PR-URL: https://github.com/nodejs/node/pull/6204
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
This allows third-party tools to check whether or not a handle that
can be unreferenced is unreferenced at a particular time.
Notably, this should be helpful for inspection via AsyncWrap.
Also, this is useful even to node's internals, particularly timers.
Refs: https://github.com/nodejs/node/pull/5828
Refs: https://github.com/nodejs/node/pull/5827
PR-URL: https://github.com/nodejs/node/pull/5834
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Record the start time so we can make the return value of Timer.now()
relative to it, increasing the chances that it fits in a tagged integer
instead of a heap-allocated double, at least for the first one or two
billion milliseconds.
PR-URL: https://github.com/nodejs/io.js/pull/2256
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Re-add the wrapper class id to AsyncWrap instances so they can be
tracked directly in a heapdump.
Previously the class id was given without setting the heap dump wrapper
class info provider. Causing a segfault when a heapdump was taken. This
has been added, and the label_ set to the given provider name so each
instance can be identified.
The id will not be set of the passed object has no internal field count.
As the class pointer cannot be retrieved from the object.
In order to properly report the allocated size of each class, the new
pure virtual method self_size() has been introduces.
PR-URL: https://github.com/nodejs/io.js/pull/1896
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Partially revert 776b73b243.
Following code crashes after backported timer leak fixes:
```javascript
var timer = setInterval(function() {
clearInterval(timer);
}, 10);
timer.unref();
```
Note that this is actually tested in a `test-timers-unref.js`, and is
crashing only with 776b73b243.
Calling `clearInterval` leads to the crashes in case of `.unref()`ed
timers, and might lead to a extra timer spin in case of regular
intervals that was closed during the interval callback. All of these
happens because `.unref()`ed timer has it's own `_handle` and was used
after the `.close()`.
PR-URL: https://github.com/iojs/io.js/pull/1330
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
Now that we are building with C++11 features enabled, replace use
of NULL with nullptr.
The benefit of using nullptr is that it can never be confused for
an integral type because it does not support implicit conversions
to integral types except boolean - unlike NULL, which is defined
as a literal `0`.
Attach the per-context execution environment directly to API functions.
Rationale:
* Gets node one step closer to multi-isolate readiness.
* Avoids multi-context confusion, e.g. when the caller and callee live
in different contexts.
* Avoids expensive calls to pthread_getspecific() on platforms where
V8 does not know how to use the thread-local storage directly.
(Linux, the BSDs.)
PR-URL: https://github.com/node-forward/node/pull/18
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Mechanically replace assert() statements with UNREACHABLE(), CHECK(),
or CHECK_{EQ,NE,LT,GT,LE,GE}() statements.
The exceptions are src/node.h and src/node_object_wrap.h because they
are public headers.
PR-URL: https://github.com/node-forward/node/pull/16
Reviewed-By: Fedor Indutny <fedor@indutny.com>
API callback functions don't need to create a v8::HandleScope instance
because V8 already creates one in the JS->C++ adapter frame.
PR-URL: https://github.com/node-forward/node/pull/16
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Calls from JS to C++ have an implicit HandleScope. So there is no need
to instantiate a new HandleScope in these basic cases.
Check if the returned int64_t is an SMI and cast the return value to
uint32_t instead of a double. Prevents needing to box the return value,
and saves a small amount of execution time.
Signed-off-by: Trevor Norris <trev.norris@gmail.com>
This prevents segfaults when a native method is reassigned to a
different object (which corrupts args.This()). When unwrapping,
clients should use args.Holder() instead of args.This().
Closes#6690.
Signed-off-by: Trevor Norris <trev.norris@gmail.com>
