src: use more appropriate context-entered check #15691
Conversation
nodejs-github-bot
added
the
c++
addaleax
force-pushed
the
async-hooks-context-check
branch
from
25b11a9
to
7a91902
Compare
addaleax
added
backport-requested-v8.x
lib / src
|
I can't say this looks correct to me. It lets you call The problem is the (No concrete proposal on what to change, just food for thought.) |
I had a similar thought, but I figured that it's not that the assertion is incorrect, IMHO it's incomplete. |
|
@bnoordhuis Do you think you’d like something along the lines of |
|
Not perfect perhaps but better. You can write it as Ideally, each context gets its own environment. That would avoid issues like #15673.
For background, it was just right when it was introduced, it's that the code base evolved afterwards. |
Make the context check in `MakeCallback` match what the comment says (and what actually makes sense). Fixes: nodejs#15672 Ref: nodejs#15428 Ref: f27b5e4
addaleax
force-pushed
the
async-hooks-context-check
branch
from
7a91902
to
c1c5862
Compare
|
@bnoordhuis I’ve updated with that. I see why you might not really be a fan, and if you prefer a much more comprehensive rewrite as a solution that should be fine, but imho this is just fine as a “does not crash anymore” kind of solution. |
|
Landed in 037d908 |
Make the context check in `MakeCallback` match what the comment says (and what actually makes sense). PR-URL: #15691 Fixes: #15672 Ref: #15428 Ref: f27b5e4 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Make the context check in `MakeCallback` match what the comment says (and what actually makes sense). PR-URL: nodejs/node#15691 Fixes: nodejs/node#15672 Ref: nodejs/node#15428 Ref: f27b5e4bdaafc73a830a0451ee3c641b8bcd08fe Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Make the context check in `MakeCallback` match what the comment says (and what actually makes sense). PR-URL: nodejs#15691 Fixes: nodejs#15672 Ref: nodejs#15428 Ref: f27b5e4 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Make the context check in `MakeCallback` match what the comment says (and what actually makes sense). PR-URL: #15691 Fixes: #15672 Ref: #15428 Ref: f27b5e4 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Make the context check in `MakeCallback` match what the comment says (and what actually makes sense). PR-URL: #15691 Fixes: #15672 Ref: #15428 Ref: f27b5e4 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
|
should this land on v6.x |
|
ping @addaleax |
Make the context check in
MakeCallbackmatch what the comment says(and what actually makes sense).
Fixes: #15672
Ref: #15428
Ref: f27b5e4
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
src