-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
Abort when using --trace-events-enabled on test-http2-respond-with-file #19921
Comments
@addaleax ... appears to happen only with the respond-with-file tests |
Adding the handle scopes should be enough, I think? diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h
index c9f123332430..f9c709aa21e9 100644
--- a/src/async_wrap-inl.h
+++ b/src/async_wrap-inl.h
@@ -50,6 +50,7 @@ inline AsyncWrap::AsyncScope::AsyncScope(AsyncWrap* wrap)
Environment* env = wrap->env();
if (env->async_hooks()->fields()[Environment::AsyncHooks::kBefore] == 0)
return;
+ v8::HandleScope handle_scope(env->isolate());
EmitBefore(env, wrap->get_async_id());
}
@@ -57,6 +58,7 @@ inline AsyncWrap::AsyncScope::~AsyncScope() {
Environment* env = wrap_->env();
if (env->async_hooks()->fields()[Environment::AsyncHooks::kAfter] == 0)
return;
+ v8::HandleScope handle_scope(env->isolate());
EmitAfter(env, wrap_->get_async_id());
}
|
I think so, yes :-) |
I haven't checked yet but it's simple to test :-) |
ryzokuken
added a commit
to ryzokuken/node
that referenced
this issue
Apr 12, 2018
Add `HandleError`s to the AsyncScope constructor and destructor in async_wrap-inl.h to fix "FATAL ERROR" incurring observed while running test-http2-respond-with-file using the --trace-events-enabled flag. Fixes: nodejs#19921
jasnell
pushed a commit
that referenced
this issue
Apr 16, 2018
Add `HandleError`s to the AsyncScope constructor and destructor in async_wrap-inl.h to fix "FATAL ERROR" incurring observed while running test-http2-respond-with-file using the --trace-events-enabled flag. Fixes: #19921 PR-URL: #19972 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ping @addaleax @AndreasMadsen @nodejs/diagnostics
The text was updated successfully, but these errors were encountered: