Use `//` for multline comments.
PR-URL: https://github.com/nodejs/node/pull/35485
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Enforce `//` for multiline comments. Some tests mixed and matched, and
at least one did so in a (to me) surprising way.
PR-URL: https://github.com/nodejs/node/pull/35485
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
When a process exits cleanly, i.e. because the event loop ends up
without things to wait for, the Node.js objects that are left on
the heap should be:
1. weak, i.e. ready for garbage collection once no longer
referenced, or
2. detached, i.e. scheduled for destruction once no longer
referenced, or
3. an unrefed libuv handle, i.e. does not keep the event loop
alive, or
4. an inactive libuv handle (essentially the same here)
There are a few exceptions to this rule, but generally,
if there are C++-backed Node.js objects on the heap
that do not fall into the above categories, we may be looking
at a potential memory leak. Most likely, the cause is a missing
`MakeWeak()` call on the corresponding object.
In order to avoid this kind of problem, we check the list
of BaseObjects for these criteria. In this commit, we only do so
when explicitly instructed to or when in debug mode
(where --verify-base-objects is always-on).
In particular, this avoids the kinds of memory leak issues
that were fixed in the PRs referenced below.
Refs: https://github.com/nodejs/node/pull/35488
Refs: https://github.com/nodejs/node/pull/35487
Refs: https://github.com/nodejs/node/pull/35481
PR-URL: https://github.com/nodejs/node/pull/35490
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/35529
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Create weak `WriteWrap` and `ShutdownWrap` objects, and when
referencing them in C++ is necessary, use `BaseObjectPtr<>`
instead of plain pointers to keep these objects from being
garbage-collected.
This solves issues that arise when the underlying `StreamBase`
instance is weak, but the `WriteWrap` or `ShutdownWrap` instances
are not; in that case, they would otherwise potentially stick
around in memory after the stream that they originally belong
to is long gone.
It probably makes sense to use `BaseObjectptr<>` more extensively
in `StreamBase` in the long run as well.
PR-URL: https://github.com/nodejs/node/pull/35488
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Once an `FSReqCallback` instance is created, it is a GC root until
the underlying fs operation has completed, meaning that it cannot
be garbage collected.
This is a problem when the underlying operation never starts
because an exception is thrown before that happens, for example
as part of parameter validation.
Instead, move all potentially throwing code before the `FSReqCallback`
creation.
PR-URL: https://github.com/nodejs/node/pull/35487
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
`GetProcessTitle()` otherwise runs an infinite loop when
`uv_setup_argv()` has not been called (yet). This is a problem
e.g. in assertions from static constructors, which run before
`main()` and thus before `argc` and `argv` become available.
To solve that, do not allocate more than 1MB of storage for the
title and bail out if we reach that point.
PR-URL: https://github.com/nodejs/node/pull/35492
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/35479
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Documents which versions of Node.js support which ESM-feature.
PR-URL: https://github.com/nodejs/node/pull/35395
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Alliased buffer header is not imported in the `env.h` file due to which
the language server is giving an error.
Fixed it via importing the header file for "aliased_buffer.h"
PR-URL: https://github.com/nodejs/node/pull/35436
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/35417
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Signed-off-by: Stewart Addison <sxa@uk.ibm.com>
PR-URL: https://github.com/nodejs/node/pull/35442
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Ash Cripps <acripps@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/35413
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Specify how to detect end-of-file when using `fs.read()` and
`filehandle.read()`.
PR-URL: https://github.com/nodejs/node/pull/35445
Fixes: https://github.com/nodejs/node/issues/35363
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
`request.connection` and `response.connection` are deprecated. Use
`request.socket` and `response.socket` instead.
PR-URL: https://github.com/nodejs/node/pull/35439
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Harshitha K P <harshitha014@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This call can fail, we should not use `.ToLocalChecked()` here.
In the long run, we should use variants of `EmitExit()` and
`EmitBeforeExit()` that avoid this problem by properly propagating
empty `MaybeLocal`s.
Example failure:
21:07:17 not ok 2564 parallel/test-worker-terminate-source-map
21:07:17 ---
21:07:17 duration_ms: 0.385
21:07:17 severity: crashed
21:07:17 exitcode: -6
21:07:17 stack: |-
21:07:17 FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.
21:07:17 1: 0x1012f04a5 node::Abort() (.cold.1) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
21:07:17 2: 0x1000b52d9 node::Abort() [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
21:07:17 3: 0x1000b543f node::OnFatalError(char const*, char const*) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
21:07:17 4: 0x1001ffd70 v8::V8::ToLocalEmpty() [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
21:07:17 5: 0x1000079b8 node::EmitBeforeExit(node::Environment*) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
21:07:17 6: 0x100156fea node::worker::Worker::Run() [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
21:07:17 7: 0x10015a4d2 node::worker::Worker::StartThread(v8::FunctionCallbackInfo<v8::Value> const&)::$_3::__invoke(void*) [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1015/out/Release/node]
21:07:17 8: 0x7fff72fe7109 _pthread_start [/usr/lib/system/libsystem_pthread.dylib]
21:07:17 9: 0x7fff72fe2b8b thread_start [/usr/lib/system/libsystem_pthread.dylib]
21:07:17 ...
PR-URL: https://github.com/nodejs/node/pull/35484
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
These objects don’t hold any resources on the event loop, so they
should be weak objects that can be garbage collected when nothing
refers to them anymore.
PR-URL: https://github.com/nodejs/node/pull/35481
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Since PR https://github.com/nodejs/node/pull/29251 is merged into
master, the implementation of `console.timeEnd` is not to print the
milliseconds on the stdout but instead print a more readable time unit
on the console. For example if the total time comes out to be 3456
milliseconds, it will be printed as 3.45 seconds so that it is much more
readable and consistent.
PR-URL: https://github.com/nodejs/node/pull/35027
Fixes: https://github.com/nodejs/node/issues/35011
Refs: https://github.com/nodejs/node/pull/29251
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
