- Check if the watcher is active in JS land before
invoking the binding, act as a noop if the state of
the watcher does not match the expectation. This
avoids firing 'stop' when the watcher is already
stopped.
- Update comments, validate more arguments and
the type of the handle.
- Handle the errors from uv_fs_poll_start
- Create an `IsActive` helper method on StatWatcher
PR-URL: https://github.com/nodejs/node/pull/19345
Refs: https://github.com/nodejs/node/pull/19089
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
In the async case, if the buffer was copied instead of being moved
then the buf will not get deleted after the request is done.
This was introduced when the FSReqWrap:: Ownership was
removed in 4b9ba9b, and ReleaseEarly was no longer called upon
destruction of FSReqWrap.
Create a custom Init function so we can use the MaybeStackBuffer in
the FSReqBase to copy the string in the async case. The data will
then get destructed when the FSReqBase is destructed.
Fixes: https://github.com/nodejs/node/issues/19356
PR-URL: https://github.com/nodejs/node/pull/19357
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently, there is an AsyncWrap constructor that is only used by
PromiseWrap. This constructor has a body which is very similar
to the other AsyncWrap constructor.
This commit suggests updating the private constructor that is used
by PromiseWrap and also have the second constructor delegate to this
one to avoid the code duplication.
PR-URL: https://github.com/nodejs/node/pull/19366
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Currently PromiseHook extracts the PromiseWrap from a Local<Promise> in
two places. This commit extracts that code into a function instead.
PR-URL: https://github.com/nodejs/node/pull/19340
Reviewed-By: James M Snell <jasnell@gmail.com>
- Make the HandleCleanup a struct, and make the queue a
std::list, iterate over it in CleanupHandles() and clear
it after that.
- Put the handle cleanup registration into a method and
document that they will not be called in the one environemt
per process setup.
PR-URL: https://github.com/nodejs/node/pull/19319
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
CleanupHandles() has not been called in our own code base anymore
after the v8 debug agent has been removed. It used to be in the
~Environment() destructor but then removed to avoid firing other
events after the exit event, given that we were not going to
clean up handles for the one environment per process setup.
Call it in FreeEnvironment so that embedders can clean up
the handles in the loop when creating multiple environments.
PR-URL: https://github.com/nodejs/node/pull/19319
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The `StreamBase` interface changed, so that `OnStreamRead()`
and `OnStreamAlloc()` are not guaranteed to be emitted in the
same tick any more.
This means that, while it isn’t causing any trouble right now,
we should not assume that it’s safe to return a static buffer
in the HTTP parser’s `OnStreamAlloc()` method.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This resolves the issue of using synchronous I/O for
`respondWithFile()` and `respondWithFD()`, and enables
scenarios in which the underlying file does not need
to be a regular file.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Add `AsyncScope` for cases where the async_hooks `before` and
`after` callbacks should be called, to track async context,
but no actual JS is called in between and we can therefore
skip things like draining the microtask or `nextTick` queues.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This enables accessing files using a more standard pattern.
Once some more refactoring has been performed on the other existing
`StreamBase` streams, this could also be used to implement `fs`
streams in a more standard manner.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Put `HandleScope`s and `Context::Scope`s where they are used,
and don’t create one for native stream callbacks automatically.
This is slightly less convenient but means that stream listeners
that don’t actually call back into JS don’t have to pay the
(small) cost of setting these up.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Currently EmitBefore, EmitAfter, EmitPromiseResolve are very similar.
This commit suggests extracting the code they have in common to a new
function to reduce code duplication.
PR-URL: https://github.com/nodejs/node/pull/19295
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Currently, BeforeExitCallback and AtExitCallback are identical apart for
the name of the struct. This commit introduces an ExitCallback struct
with can be used in both cases to avoid the duplication.
PR-URL: https://github.com/nodejs/node/pull/19226
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently there is no member initialiser for emit_env_nonstring_warning_
in the Environment constructor leading to undefined behaviour.
For a debug build, this memory would be initialized:
(lldb) memory read -f x -s 4 -c 1 &emit_env_nonstring_warning_
0x7fff5fbfe254: 0x00000001
But for a release build there will be no such initialization:
(lldb) memory read -f x -s 4 -c 1 `$r12 + 0x46c`
0x7fff5fbfe374: 0x00000000
This can be seen by running test-process-env-deprecation.js using
the Debug build and the Release build.
This commit adds a member initialiser for emit_env_nonstring_warning_
setting it to true.
PR-URL: https://github.com/nodejs/node/pull/19283
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Replace the `data_` member of `FSReqBase` by a boolean flag, because
the pointer that `data()` returns is already available through the
`buffer_` member, and set the default size for not requiring an
extra allocation to a more reasonable value, as the input is
usually something like a file path.
PR-URL: https://github.com/nodejs/node/pull/19174
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Adds `TRACING_CATEGORY_NODE`, `TRACING_CATEGORY_NODE1` and
`TRACING_CATEGORY_NODE2` helper macros for consistently building
trace event category strings. For instance,
`TRACING_CATEGORY_NODE2(foo, bar)` would generate the category
string `node,node.foo,node.foo.bar`, such that...
```
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
TRACING_CATEGORY_NODE2(foo, bar),
"baz", 1);
```
Would emit if trace events are enabled for categories: `node`,
`node.foo`, or `node.foo.bar`.
This allows a natural scoping down of what trace events a user
may want to receive. Enabling the `node` category would receive
everything Node.js produces.
PR-URL: https://github.com/nodejs/node/pull/19155
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Tobias Nießen