Skip to content
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.

Sign up for GitHub

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

Jump to bottom

http2: assertion failed #21008

Closed
ronag opened this issue May 28, 2018 · 2 comments
Closed

http2: assertion failed #21008

ronag opened this issue May 28, 2018 · 2 comments
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.

Comments

@ronag
Copy link
Member

ronag commented May 28, 2018

Node 10.2.1, OSX

/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node[47055]: ../src/node_http2.cc:1819:void node::http2::Http2Stream::OnTrailers(): Assertion `!this->IsDestroyed()' failed.
 1: node::Abort() [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 2: node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, char const*, int, v8::Local<v8::Value>*, node::async_context) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 3: node::http2::Http2Stream::OnTrailers() [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 4: node::http2::Http2Stream::Provider::Stream::OnRead(nghttp2_session*, int, unsigned char*, unsigned long, unsigned int*, nghttp2_data_source*, void*) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 5: nghttp2_session_pack_data [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 6: nghttp2_session_mem_send_internal [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 7: nghttp2_session_mem_send [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 8: node::http2::Http2Session::SendPendingData() [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
 9: node::http2::Http2Session::ClearOutgoing(int) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
10: node::http2::Http2Session::OnStreamAfterWrite(node::WriteWrap*, int) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
11: node::WriteWrap::OnDone(int) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
12: node::Environment::RunAndClearNativeImmediates() [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
13: node::Environment::CheckImmediate(uv_check_s*) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
14: uv__run_check [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
15: uv_run [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
16: node::Start(v8::Isolate*, node::IsolateData*, int, char const* const*, int, char const* const*) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
17: node::Start(uv_loop_s*, int, char const* const*, int, char const* const*) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
18: node::Start(int, char**) [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
19: start [/Users/ronagy/.nvm/versions/node/v10.2.1/bin/node]
@ronag ronag changed the title assertion failed http2: assertion failed May 28, 2018
@bnoordhuis bnoordhuis added the http2 Issues or PRs related to the http2 subsystem. label May 29, 2018
@apapirovski
Copy link
Member

Thanks for reporting @ronag. I think the CHECK that is failing here is actually incorrect as things are. I think we need to avoid calling OnTrailers if the stream is already destroyed because it's possible for OnRead to trigger after that's already the case.

@apapirovski apapirovski added the confirmed-bug Issues with confirmed bugs. label May 29, 2018
@apapirovski apapirovski mentioned this issue May 29, 2018
Closed
3 tasks
@apapirovski
Copy link
Member

Was a bit more complicated than I had initially assumed. Fix in #21016

addaleax pushed a commit that referenced this issue Jun 1, 2018
@apapirovski @addaleax
http2: force through RST_STREAM in destroy
d4787cf 
If still needed, force through RST_STREAM in Http2Stream#destroy
calls, so that nghttp2 can wrap up properly and doesn't continue
trying to read & write data to the stream.

PR-URL: #21016
Fixes: #21008
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
kjin pushed a commit to kjin/node that referenced this issue Aug 23, 2018
@apapirovski @kjin
http2: force through RST_STREAM in destroy
1322bea 
If still needed, force through RST_STREAM in Http2Stream#destroy
calls, so that nghttp2 can wrap up properly and doesn't continue
trying to read & write data to the stream.

PR-URL: nodejs#21016
Fixes: nodejs#21008
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
kjin pushed a commit to kjin/node that referenced this issue Sep 25, 2018
@apapirovski @kjin
http2: force through RST_STREAM in destroy
f42f505 
If still needed, force through RST_STREAM in Http2Stream#destroy
calls, so that nghttp2 can wrap up properly and doesn't continue
trying to read & write data to the stream.

PR-URL: nodejs#21016
Fixes: nodejs#21008
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
kjin pushed a commit to kjin/node that referenced this issue Oct 16, 2018
@apapirovski @kjin
http2: force through RST_STREAM in destroy
21f3e27 
If still needed, force through RST_STREAM in Http2Stream#destroy
calls, so that nghttp2 can wrap up properly and doesn't continue
trying to read & write data to the stream.

PR-URL: nodejs#21016
Fixes: nodejs#21008
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
BethGriggs pushed a commit that referenced this issue Oct 17, 2018
@apapirovski @BethGriggs
http2: force through RST_STREAM in destroy
b22266c 
If still needed, force through RST_STREAM in Http2Stream#destroy
calls, so that nghttp2 can wrap up properly and doesn't continue
trying to read & write data to the stream.

Backport-PR-URL: #22850
PR-URL: #21016
Fixes: #21008
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http2: force through RST_STREAM in destroy apapirovski/node
3 participants
@apapirovski @bnoordhuis @ronag