quic: fix push stream closing

PR-URL: #341 Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs · Feb 12, 2020 · 2781829 · 2781829
1 parent 32c587d
commit 2781829
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 8 deletions.
diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js
@@ -2408,7 +2408,8 @@ class QuicStream extends Duplex {
       ...options,
       allowHalfOpen: true,
       decodeStrings: true,
-      emitClose: true
+      emitClose: true,
+      autoDestroy: false,
     });
     this.#session = session;
     this.#push_id = push_id;

diff --git a/src/quic/node_quic_default_application.cc b/src/quic/node_quic_default_application.cc
@@ -67,6 +67,8 @@ void DefaultApplication::UnscheduleStream(int64_t stream_id) {
 void DefaultApplication::StreamClose(
     int64_t stream_id,
     uint64_t app_error_code) {
+  if (!session()->HasStream(stream_id))
+    return;
   if (app_error_code == 0)
     app_error_code = NGTCP2_APP_NOERROR;
   UnscheduleStream(stream_id);

diff --git a/src/quic/node_quic_http3_application.cc b/src/quic/node_quic_http3_application.cc
@@ -682,9 +682,12 @@ void Http3Application::ReceiveData(
 void Http3Application::DeferredConsume(
     int64_t stream_id,
     size_t consumed) {
-  BaseObjectPtr<QuicStream> stream = session()->FindStream(stream_id);
-  CHECK(stream);
-  session()->ExtendStreamOffset(stream->id(), consumed);
+  // Do nothing here for now. nghttp3 uses the on_deferred_consume
+  // callback to notify when stream data that had previously been
+  // deferred has been delivered to the application so that the
+  // stream data offset can be extended. However, we extend the
+  // data offset from within QuicStream when the data is delivered
+  // so we don't have to do it here.
 }
 
 // Called when a nghttp3 detects that a new block of headers

diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc
@@ -1264,7 +1264,7 @@ void QuicApplication::MaybeSetFin(const StreamData& stream_data) {
 }
 
 void QuicApplication::StreamOpen(int64_t stream_id) {
-  Debug(session(), "QUIC Stream %" PRId64 " is open.");
+  Debug(session(), "QUIC Stream %" PRId64 " is open.", stream_id);
 }
 
 void QuicApplication::StreamHeaders(
@@ -2385,9 +2385,7 @@ void QuicSession::StreamClose(int64_t stream_id, uint64_t app_error_code) {
         stream_id,
         app_error_code);
 
-  // If the stream does not actually exist, just ignore
-  if (HasStream(stream_id))
-    application_->StreamClose(stream_id, app_error_code);
+  application_->StreamClose(stream_id, app_error_code);
 }
 
 // Called by ngtcp2 when a stream has been opened. All we do is log