delta/node-new.git/src/timer_wrap.cc, branch devsnek-patch-1

src: guard against nullptr deref in TimerWrapHandle::Stop

2020-07-21T19:11:16+00:00

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

PR-URL: https://github.com/nodejs/node/pull/34460
Reviewed-By: Jiawen Geng 
Reviewed-By: Richard Lau 
Reviewed-By: James M Snell 
Reviewed-By: David Carlier

src: refactor TimerWrap lifetime management

2020-07-14T13:04:46+00:00

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 
Reviewed-By: Zeyu Yang 
Reviewed-By: David Carlier

src: remove user_data from TimerWrap

2020-07-14T13:04:43+00:00

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 
Reviewed-By: Zeyu Yang 
Reviewed-By: David Carlier

src: add TimerWrap utility

2020-07-05T21:56:59+00:00

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 

PR-URL: https://github.com/nodejs/node/pull/34186
Reviewed-By: Anna Henningsen 
Reviewed-By: Matteo Collina

src: remove defunct timer_wrap file

2018-07-15T13:29:45+00:00

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. ([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 
Reviewed-By: James M Snell 
Reviewed-By: Tiancheng "Timothy" Gu 
Reviewed-By: Ruben Bridgewater 
Reviewed-By: Gus Caplan 
Reviewed-By: Colin Ihrig 
Reviewed-By: Luigi Pinca 
Refs: https://github.com/nodejs/node/pull/20894

src: enable more detailed memory tracking

2018-07-13T17:53:15+00:00

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

src: add HandleWrap::AddWrapMethods

2018-07-13T16:08:20+00:00

Extracts common setters to a single location

PR-URL: https://github.com/nodejs/node/pull/21769
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Minwoo Jung 
Reviewed-By: Ruben Bridgewater 
Reviewed-By: Joyee Cheung

src: break out of timers loop if `!can_call_into_js()`

2018-05-31T09:53:30+00:00

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 
Reviewed-By: Anatoli Papirovski 
Reviewed-By: Colin Ihrig 
Reviewed-By: James M Snell 
Reviewed-By: Tobias Nießen

timers: use only a single TimerWrap instance

2018-05-22T19:26:12+00:00

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 
Reviewed-By: Jeremiah Senkpiel 
Reviewed-By: Matteo Collina 
Reviewed-By: James M Snell 
Reviewed-By: Ruben Bridgewater

src: refactor `BaseObject` internal field management

2018-05-03T22:57:39+00:00

- 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
  ddc19be6de1ba263d9c175b2760696e7b9918b25) 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 ddc19be6de1ba263d9c175b2760696e7b9918b25, 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 
Reviewed-By: Ben Noordhuis 
Reviewed-By: Daniel Bevenius