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.

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

nodejs v8.11.3 aborts in http2 with TLS if client closes session #22923

Closed
derolf opened this issue Sep 18, 2018 · 2 comments
Closed

nodejs v8.11.3 aborts in http2 with TLS if client closes session #22923

derolf opened this issue Sep 18, 2018 · 2 comments
Labels
http2 Issues or PRs related to the http2 subsystem. tls Issues and PRs related to the tls subsystem.

Comments

@derolf
Copy link

derolf commented Sep 18, 2018

node: v8.11.3
system: Ubuntu 18.04

I am hitting an issue with nodejs v8.11.3. I am using http2 with TLS (https) and the server aborts when the client has closed the session. Here is the error:

HTTP2 31334: Http2Session server: socket closed
HTTP2 31334: Http2Session server: marking session closed
HTTP2 31334: Http2Session server: submitting goaway
node[31334]: ../src/tls_wrap.cc:604:virtual int node::TLSWrap::DoWrite(node::WriteWrap*, uv_buf_t*, size_t, uv_stream_t*): Assertion `(ssl_) != (nullptr)' failed.
 1: node::Abort() [node]
 2: 0x8c25db [node]
 3: node::TLSWrap::DoWrite(node::WriteWrap*, uv_buf_t*, unsigned long, uv_stream_s*) [node]
 4: node::http2::Http2Session::SendPendingData() [node]
 5: 0x90e769 [node]
 6: node::Environment::RunAndClearNativeImmediates() [node]
 7: node::Environment::CheckImmediate(uv_check_s*) [node]
 8: 0x141a4ac [node]
 9: uv_run [node]
10: node::Start(uv_loop_s*, int, char const* const*, int, char const* const*) [node]
11: node::Start(int, char**) [node]
12: __libc_start_main [/lib/x86_64-linux-gnu/libc.so.6]
13: 0x89b1b1 [node]
Aborted (core dumped)

What confuses me is why does the server still emit a GOAWAY frame although the socket already was closed?

Note: the problem does not always happen, but is reproducible as part of a more complex test scenario.

QUIRK SOLUTION

I made a quirk to circumvent the problem by monkeypatching "goaway" and introduction an explicit check if the socket has been closed. TypeScript code:

const GOAWAY = Symbol();

interface ExtServerHttp2Session extends http2.ServerHttp2Session {
  [GOAWAY]?: (code?: number, lastStreamID?: number, opaqueData?: Buffer | DataView /*| TypedArray*/) => void;
}

function patchedGoaway(
  this: ExtServerHttp2Session,
  code?: number,
  lastStreamID?: number,
  opaqueData?: Buffer | DataView /*| TypedArray*/
): void {
  if (!this.closed) {
    this[GOAWAY]!(code, lastStreamID, opaqueData);
  }
}

function monkeyPatch(session: http2.Http2Session) {
  const extSession = session as ExtServerHttp2Session;
  if (!extSession[GOAWAY]) {
    extSession[GOAWAY] = extSession.goaway;
    extSession.goaway = patchedGoaway;
  }
}

Now when handling a new stream, you call monkeyPatch(stream.session).

@addaleax addaleax added tls Issues and PRs related to the tls subsystem. v8.x http2 Issues or PRs related to the http2 subsystem. labels Sep 18, 2018
@addaleax
Copy link
Member

I think a combination of #22924 and #22850 will probably fully address this.

@apapirovski
Copy link
Member

I'm reasonably certain this has been addressed with all the backports. Feel free to reopen if you think this is an ongoing issue or you happen to have more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants