[v14.x] backport process.exit changes by addaleax · Pull Request #38786 · nodejs/node

added 3 commits

May 23, 2021 16:37
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: nodejs#35488
Refs: nodejs#35487
Refs: nodejs#35481

PR-URL: nodejs#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>
     ../src/env.cc:1227:22: error: lambda capture 'this' is not used [-Werror,-Wunused-lambda-capture]
      ForEachBaseObject([this](BaseObject* obj) {
                     ^~~~

PR-URL: nodejs#35547
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: nodejs#35486
Reviewed-By: James M Snell <jasnell@gmail.com>

@github-actions github-actions bot added c++

Issues and PRs that require attention from people who are familiar with C++.

lib / src

Issues and PRs related to general changes in the lib or src directory.

needs-ci

PRs that need a full CI run.

v14.x labels

May 23, 2021

targos pushed a commit that referenced this pull request

May 25, 2021
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: #35488
Refs: #35487
Refs: #35481

PR-URL: #35490
Backport-PR-URL: #38786
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>

targos pushed a commit that referenced this pull request

May 25, 2021
     ../src/env.cc:1227:22: error: lambda capture 'this' is not used [-Werror,-Wunused-lambda-capture]
      ForEachBaseObject([this](BaseObject* obj) {
                     ^~~~

PR-URL: #35547
Backport-PR-URL: #38786
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

targos pushed a commit that referenced this pull request

May 25, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

targos pushed a commit that referenced this pull request

Jun 6, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

targos pushed a commit that referenced this pull request

Jun 6, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

targos pushed a commit that referenced this pull request

Jun 11, 2021
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: #35488
Refs: #35487
Refs: #35481

PR-URL: #35490
Backport-PR-URL: #38786
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>

targos pushed a commit that referenced this pull request

Jun 11, 2021
     ../src/env.cc:1227:22: error: lambda capture 'this' is not used [-Werror,-Wunused-lambda-capture]
      ForEachBaseObject([this](BaseObject* obj) {
                     ^~~~

PR-URL: #35547
Backport-PR-URL: #38786
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

targos pushed a commit that referenced this pull request

Jun 11, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

targos pushed a commit that referenced this pull request

Jun 15, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

richardlau pushed a commit that referenced this pull request

Jul 5, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

richardlau pushed a commit that referenced this pull request

Jul 29, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

BethGriggs pushed a commit that referenced this pull request

Aug 12, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Aug 31, 2021
This addresses a TODO comment, and removes invalid `.ToLocalChecked()`
calls from our code base.

PR-URL: #35486
Backport-PR-URL: #38786
Reviewed-By: James M Snell <jasnell@gmail.com>