Node.js HTTP/2 segfault if underlying socket is unexpectedly closed

[pimterry]

Version

On main. Also fails on Node 14.21.2, 16.20.1, 18.17.1 & 20.3.1

Platform

Linux 5.19.0-46-generic x86_64

Subsystem

http2

What steps will reproduce the bug?

Here's a ready-to-go failing test - drop it into test/parallel and run:

'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
if (!common.hasCrypto)
  common.skip('missing crypto');
const net = require('net');
const h2 = require('http2');

const tlsOptions = {
  key: fixtures.readKey('agent1-key.pem'),
  cert: fixtures.readKey('agent1-cert.pem'),
  ALPNProtocols: ['h2']
};

const netServer = net.createServer((socket) => {
  setTimeout(() => {
    socket.destroy();
  }, 10);

  h2Server.emit('connection', socket);
});

const h2Server = h2.createSecureServer(tlsOptions);

h2Server.on('session', session => {
  setTimeout(() => {
    session.destroy();
  }, 20);
});

netServer.listen(0, common.mustCall(() => {
  h2.connect(`https://localhost:${netServer.address().port}`, {
    rejectUnauthorized: false
  });
}));

How often does it reproduce? Is there a required condition?

Fails every time.

What is the expected behavior? Why is that the expected behavior?

The session should just close I think. Maybe there should be an error here, but it definitely shouldn't segfault.

What do you see instead?

This test crashes with a segmentation fault. The backtrace is:

#0  0x0000555555ff8b1e in node::StreamBase::Write(uv_buf_t*, unsigned long, uv_stream_s*, v8::Local<v8::Object>, bool) ()
#1  0x00005555560c6a12 in node::crypto::TLSWrap::EncOut() ()
#2  0x00005555560c87b4 in node::crypto::TLSWrap::DoWrite(node::WriteWrap*, uv_buf_t*, unsigned long, uv_stream_s*) ()
#3  0x0000555555ff8c8d in node::StreamBase::Write(uv_buf_t*, unsigned long, uv_stream_s*, v8::Local<v8::Object>, bool) ()
#4  0x0000555555f19560 in node::http2::Http2Session::SendPendingData() ()
#5  0x0000555555f19eaf in node::http2::Http2Session::Close(unsigned int, bool) ()
#6  0x00005555561bc160 in v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) ()
#7  0x00005555561bc6f8 in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, unsigned long*, int) ()
#8  0x00005555561bcf28 in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) ()
#9  0x0000555556c26df6 in Builtins_CEntry_Return1_ArgvOnStack_BuiltinExit ()
#10 0x0000555556b98d1c in Builtins_InterpreterEntryTrampoline ()

Additional information

This is a minimal version I think, but similar behaviour will occur in real-world setups with requests and responses here too. In effect, situations where socket & session are idle, and then the underlying socket closes before the HTTP/2 session does can result in a segfault.

I'm not sure why, but this requires TLS here - that means that actual stack of streams is net.Socket + TLSSocket + HTTP/2 session, and we're destroying the net.Socket first, and then the HTTP/2 session.

I wouldn't be surprised if there are other cases without using the separate socket server here to trigger this, where actual network traffic results in this same crash, but I can't confirm that yet.

I ran into this trying to explore & fix #46094, where one comment describes that error coming from a very similar flow. I haven't managed to reliably reproduce the exception described in that issue, but this segfault is definitely a problem too! There may be a relationship to that issue anyway, but it's hard to say for sure.