[v9.x] backport 18656 + 19002 #19185

addaleax · 2018-03-06T22:43:21Z

This should go into the next v9.x release, since #19002 is a somewhat serious bug fix that a few people have been running into

Backport:

@MylesBorins

Not going to kick off CI because v9.x-staging is broken right now anyway.

PR-URL: nodejs#18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

Replace v8::Persistent with node::Persistent, a specialization that resets the persistent handle on destruction. Prevents accidental resource leaks when forgetting to call .Reset() manually. I'm fairly confident this commit fixes a number of resource leaks that have gone undiagnosed so far. PR-URL: nodejs#18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

The previous commit made persistent handles auto-reset on destruction. This commit removes the Reset() calls that are now no longer necessary. PR-URL: nodejs#18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Don't try to update the internal field pointer of the JS object in the destructor. The garbage collector invokes the destructor when the object is collected and is not necessarily in a valid state anymore. PR-URL: nodejs#18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. PR-URL: nodejs#19002 Fixes: nodejs#18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

Replace v8::Persistent with node::Persistent, a specialization that resets the persistent handle on destruction. Prevents accidental resource leaks when forgetting to call .Reset() manually. I'm fairly confident this commit fixes a number of resource leaks that have gone undiagnosed so far. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

The previous commit made persistent handles auto-reset on destruction. This commit removes the Reset() calls that are now no longer necessary. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Don't try to update the internal field pointer of the JS object in the destructor. The garbage collector invokes the destructor when the object is collected and is not necessarily in a valid state anymore. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. Backport-PR-URL: #19185 PR-URL: #19002 Fixes: #18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

MylesBorins · 2018-03-06T23:02:18Z

landed in cd71a52...3038c9c

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

Replace v8::Persistent with node::Persistent, a specialization that resets the persistent handle on destruction. Prevents accidental resource leaks when forgetting to call .Reset() manually. I'm fairly confident this commit fixes a number of resource leaks that have gone undiagnosed so far. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

The previous commit made persistent handles auto-reset on destruction. This commit removes the Reset() calls that are now no longer necessary. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Don't try to update the internal field pointer of the JS object in the destructor. The garbage collector invokes the destructor when the object is collected and is not necessarily in a valid state anymore. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. Backport-PR-URL: #19185 PR-URL: #19002 Fixes: #18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

Replace v8::Persistent with node::Persistent, a specialization that resets the persistent handle on destruction. Prevents accidental resource leaks when forgetting to call .Reset() manually. I'm fairly confident this commit fixes a number of resource leaks that have gone undiagnosed so far. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

The previous commit made persistent handles auto-reset on destruction. This commit removes the Reset() calls that are now no longer necessary. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Don't try to update the internal field pointer of the JS object in the destructor. The garbage collector invokes the destructor when the object is collected and is not necessarily in a valid state anymore. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. Backport-PR-URL: #19185 PR-URL: #19002 Fixes: #18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

Replace v8::Persistent with node::Persistent, a specialization that resets the persistent handle on destruction. Prevents accidental resource leaks when forgetting to call .Reset() manually. I'm fairly confident this commit fixes a number of resource leaks that have gone undiagnosed so far. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

The previous commit made persistent handles auto-reset on destruction. This commit removes the Reset() calls that are now no longer necessary. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

Don't try to update the internal field pointer of the JS object in the destructor. The garbage collector invokes the destructor when the object is collected and is not necessarily in a valid state anymore. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. Backport-PR-URL: #19185 PR-URL: #19002 Fixes: #18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. Backport-PR-URL: nodejs#19185 PR-URL: nodejs#19002 Fixes: nodejs#18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

This fixes a crash that occurred when a `Http2Stream` write is completed after it is already destroyed. Instead, don’t destroy the stream in that case and wait for GC to take over. Backport-PR-URL: #19185 Backport-PR-URL: #20456 PR-URL: #19002 Fixes: #18973 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

jasnell and others added 6 commits March 6, 2018 23:35

src: handle exceptions in env->SetImmediates

8ff43ad

PR-URL: nodejs#18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

[squash] v9.x-specific fixups

097009a

nodejs-github-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. v9.x labels Mar 6, 2018

addaleax mentioned this pull request Mar 6, 2018

v9.8.0 proposal #19181

Merged

MylesBorins force-pushed the v9.x-staging branch from a42c959 to 3038c9c Compare March 6, 2018 22:57

MylesBorins pushed a commit that referenced this pull request Mar 6, 2018

src: handle exceptions in env->SetImmediates

01348c8

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

MylesBorins closed this Mar 6, 2018

MylesBorins pushed a commit that referenced this pull request Mar 6, 2018

src: handle exceptions in env->SetImmediates

7acf21a

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

MylesBorins pushed a commit that referenced this pull request Mar 6, 2018

src: handle exceptions in env->SetImmediates

03784d7

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

MylesBorins pushed a commit that referenced this pull request Mar 7, 2018

src: handle exceptions in env->SetImmediates

08bcdde

Backport-PR-URL: #19185 PR-URL: #18297 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v9.x] backport 18656 + 19002 #19185

[v9.x] backport 18656 + 19002 #19185

addaleax commented Mar 6, 2018

MylesBorins commented Mar 6, 2018

[v9.x] backport 18656 + 19002 #19185

[v9.x] backport 18656 + 19002 #19185

Conversation

addaleax commented Mar 6, 2018

MylesBorins commented Mar 6, 2018