-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
src: call Environment::Exit()
for fatal exceptions
#25472
Conversation
This makes sure that `StopTracingAgent()` is always called before tearing down the `tracing::Agent`, since previously its destructor might have tried to access the agent, which would be destroyed by the (earlier) `Dispose()` call.
Call `Environment::Exit()` rather than the process-wide `exit()` function, since JS exceptions generally only affect the current JS engine instance.
@addaleax sadly an error occured when I tried to trigger a build :( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, I believe this resolves an issue I have in a local branch 👍 (it’s possible to trigger the destruction of the global native module loader through exit() from the FatalException handler of a worker thread while
other threads are still active)
Landed in 391f839...8528c21 |
This makes sure that `StopTracingAgent()` is always called before tearing down the `tracing::Agent`, since previously its destructor might have tried to access the agent, which would be destroyed by the (earlier) `Dispose()` call. PR-URL: #25472 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Call `Environment::Exit()` rather than the process-wide `exit()` function, since JS exceptions generally only affect the current JS engine instance. PR-URL: #25472 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
This makes sure that `StopTracingAgent()` is always called before tearing down the `tracing::Agent`, since previously its destructor might have tried to access the agent, which would be destroyed by the (earlier) `Dispose()` call. PR-URL: #25472 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Call `Environment::Exit()` rather than the process-wide `exit()` function, since JS exceptions generally only affect the current JS engine instance. PR-URL: #25472 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Tests fail without the first commit, so they are in one PR.
src: reset
StopTracingAgent()
before platform teardownThis makes sure that
StopTracingAgent()
is always calledbefore tearing down the
tracing::Agent
,since previously its destructor might have tried to access the
agent, which would be destroyed by the (earlier)
Dispose()
call.src: call
Environment::Exit()
for fatal exceptionsCall
Environment::Exit()
rather than the process-wideexit()
function, since JS exceptions generally only affectthe current JS engine instance.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes