src: restrict unloading addons to Worker threads by addaleax · Pull Request #25577 · nodejs/node

@nodejs-github-bot nodejs-github-bot added the c++

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

label

Jan 18, 2019

This was referenced

Jan 18, 2019

refack

refack previously approved these changes Jan 21, 2019

bnoordhuis

Unloading native addons from the main thread was an (presumably
unintended) significant breaking change, because addons may
rely on their memory being available after an `Environment` exits.

This patch only restricts this to Worker threads, at least for the
time being, and thus matches the behaviour from
nodejs#23319.

Refs: nodejs#24861
Refs: nodejs#23319

@addaleax

@addaleax addaleax added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Jan 22, 2019

danbev pushed a commit that referenced this pull request

Jan 23, 2019
Unloading native addons from the main thread was an (presumably
unintended) significant breaking change, because addons may
rely on their memory being available after an `Environment` exits.

This patch only restricts this to Worker threads, at least for the
time being, and thus matches the behaviour from
#23319.

PR-URL: #25577
Refs: #24861
Refs: #23319
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@addaleax addaleax deleted the addon-unload-main-thread branch

January 23, 2019 16:05

addaleax added a commit that referenced this pull request

Jan 23, 2019
Unloading native addons from the main thread was an (presumably
unintended) significant breaking change, because addons may
rely on their memory being available after an `Environment` exits.

This patch only restricts this to Worker threads, at least for the
time being, and thus matches the behaviour from
#23319.

PR-URL: #25577
Refs: #24861
Refs: #23319
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>