diff --git a/src/node_http2.cc b/src/node_http2.cc index 7a5e9ba23e99b3..1274bc7f5b8d4d 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2060,10 +2060,9 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap, uv_buf_t* bufs, size_t nbufs, uv_stream_t* send_handle) { - CHECK(!this->IsDestroyed()); CHECK_NULL(send_handle); Http2Scope h2scope(this); - if (!IsWritable()) { + if (!IsWritable() || IsDestroyed()) { req_wrap->Done(UV_EOF); return 0; } diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index bfe7d4297257a0..e19f98e35d2821 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -57,9 +57,11 @@ void StreamPipe::Unpipe() { if (is_closed_) return; - // Note that we cannot use virtual methods on `source` and `sink` here, - // because this function can be called from their destructors via + // Note that we possibly cannot use virtual methods on `source` and `sink` + // here, because this function can be called from their destructors via // `OnStreamDestroy()`. + if (!source_destroyed_) + source()->ReadStop(); is_closed_ = true; is_reading_ = false; @@ -144,7 +146,8 @@ void StreamPipe::ProcessData(size_t nread, const uv_buf_t& buf) { is_writing_ = true; is_reading_ = false; res.wrap->SetAllocatedStorage(buf.base, buf.len); - source()->ReadStop(); + if (source() != nullptr) + source()->ReadStop(); } } @@ -183,6 +186,7 @@ void StreamPipe::WritableListener::OnStreamAfterShutdown(ShutdownWrap* w, void StreamPipe::ReadableListener::OnStreamDestroy() { StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this); + pipe->source_destroyed_ = true; if (!pipe->is_eof_) { OnStreamRead(UV_EPIPE, uv_buf_init(nullptr, 0)); } @@ -190,6 +194,7 @@ void StreamPipe::ReadableListener::OnStreamDestroy() { void StreamPipe::WritableListener::OnStreamDestroy() { StreamPipe* pipe = ContainerOf(&StreamPipe::writable_listener_, this); + pipe->sink_destroyed_ = true; pipe->is_eof_ = true; pipe->Unpipe(); } diff --git a/src/stream_pipe.h b/src/stream_pipe.h index b72a60941b610a..36a0b1dc08106b 100644 --- a/src/stream_pipe.h +++ b/src/stream_pipe.h @@ -23,16 +23,18 @@ class StreamPipe : public AsyncWrap { } private: - StreamBase* source(); - StreamBase* sink(); + inline StreamBase* source(); + inline StreamBase* sink(); - void ShutdownWritable(); - void FlushToWritable(); + inline void ShutdownWritable(); + inline void FlushToWritable(); bool is_reading_ = false; bool is_writing_ = false; bool is_eof_ = false; bool is_closed_ = true; + bool sink_destroyed_ = false; + bool source_destroyed_ = false; // Set a default value so that when we’re coming from Start(), we know // that we don’t want to read just yet. diff --git a/test/parallel/test-http2-respond-with-file-connection-abort.js b/test/parallel/test-http2-respond-with-file-connection-abort.js new file mode 100644 index 00000000000000..25926b2c9805a3 --- /dev/null +++ b/test/parallel/test-http2-respond-with-file-connection-abort.js @@ -0,0 +1,37 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); +const net = require('net'); + +const { + HTTP2_HEADER_CONTENT_TYPE +} = http2.constants; + +const server = http2.createServer(); +server.on('stream', common.mustCall((stream) => { + stream.respondWithFile(process.execPath, { + [HTTP2_HEADER_CONTENT_TYPE]: 'application/octet-stream' + }); +})); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request(); + + req.on('response', common.mustCall(() => {})); + req.on('data', common.mustCall(() => { + net.Socket.prototype.destroy.call(client.socket); + server.close(); + })); + req.end(); +})); + +// TODO(addaleax): This is a *hack*. HTTP/2 needs to have a proper way of +// dealing with this kind of issue. +process.once('uncaughtException', (err) => { + if (err.code === 'ECONNRESET') return; + throw err; +});