http2: finish empty trailers natively for compat streams

When the compat layer flushes response headers before the response
is ended (writeHead(), write(), flushHeaders()), it must keep
waitForTrailers so that trailers can still be added while streaming.
As a result, every such response paid for a wantTrailers C++ -> JS
callback, an empty sendTrailers() with its setImmediate(), and a
trailers() call back into C++, even though most responses never
register any trailers.

Introduce STREAM_OPTION_AUTO_EMPTY_TRAILERS: when set and no
trailers have been handed to the native side by the time the final
DATA frame is sent, the stream is finished directly in C++ with the
same empty DATA frame carrying END_STREAM that the JS path would
have produced, without calling into JS at all. The compat layer
enables this mode whenever it responds with waitForTrailers and no
trailers registered yet; a later setTrailer() call flips the stream
back to JS-managed trailers through a new disableAutoTrailers()
binding, so streaming trailers keep working unchanged.

The wire format is identical in all cases.

h2load -c 4 -m 100, 1 KiB payload, mean of 8 alternating runs
against the previous commit: compat writeHead()+end() 47.8k -> 50.2k
req/s (+5.0%); multi-write streaming responses +1%.

Signed-off-by: Matteo Collina <hello@matteocollina.com>

Author: mcollina

lib/internal/http2/compat.js

@@ -52,6 +52,8 @@ const {
   validateObject,
 } = require('internal/validators');
 const {
+  kAutoEmptyTrailers,
+  kDisableAutoTrailers,
   kSocket,
   kRequest,
   kProxySocket,
@@ -592,7 +594,15 @@ class Http2ServerResponse extends Stream {
     name = name.trim().toLowerCase();
     assertValidHeader(name, value);
     this[kTrailers][name] = value;
-    this[kState].hasTrailers = true;
+    const state = this[kState];
+    if (!state.hasTrailers) {
+      state.hasTrailers = true;
+      // If the response headers were already flushed with auto-empty
+      // trailers enabled, tell the stream to hand the trailers back to JS.
+      const stream = this[kStream];
+      if (stream.headersSent)
+        stream[kDisableAutoTrailers]();
+    }
   }
 
   addTrailers(headers) {
@@ -911,13 +921,18 @@ class Http2ServerResponse extends Stream {
     const state = this[kState];
     const headers = this[kHeaders];
     headers[HTTP2_HEADER_STATUS] = state.statusCode;
+    // Only wait for trailers if some have been registered, or if the
+    // headers are flushed before the response is ended (in which case
+    // trailers may still be added during streaming). Trailers added after
+    // end() are dropped, matching HTTP/1 addTrailers() semantics.
+    const waitForTrailers = state.hasTrailers || !state.finishing;
     const options = {
       endStream: state.ending,
-      // Only wait for trailers if some have been registered, or if the
-      // headers are flushed before the response is ended (in which case
-      // trailers may still be added during streaming). Trailers added after
-      // end() are dropped, matching HTTP/1 addTrailers() semantics.
-      waitForTrailers: state.hasTrailers || !state.finishing,
+      waitForTrailers,
+      // When no trailers have been registered yet, let the native side
+      // finish the stream on its own if none show up by the time the last
+      // DATA frame is sent (see setTrailer()).
+      [kAutoEmptyTrailers]: waitForTrailers && !state.hasTrailers,
       sendDate: state.sendDate,
     };
     this[kStream].respond(headers, options);

lib/internal/http2/core.js

@@ -150,6 +150,8 @@ const {
   getStreamState,
   isPayloadMeaningless,
   kAuthority,
+  kAutoEmptyTrailers,
+  kDisableAutoTrailers,
   kSensitiveHeaders,
   kStrictSingleValueFields,
   kSocket,
@@ -334,6 +336,7 @@ const {
 
   STREAM_OPTION_EMPTY_PAYLOAD,
   STREAM_OPTION_GET_TRAILERS,
+  STREAM_OPTION_AUTO_EMPTY_TRAILERS,
 } = constants;
 
 const STREAM_FLAGS_PENDING = 0x0;
@@ -2520,6 +2523,17 @@ class Http2Stream extends Duplex {
     }
   }
 
+  // Called by the compat layer when trailers are registered after the
+  // response headers were already sent with auto-empty trailers enabled
+  // (see STREAM_OPTION_AUTO_EMPTY_TRAILERS); switches the stream back to
+  // JS-managed trailers.
+  [kDisableAutoTrailers]() {
+    const handle = this[kHandle];
+    if (this.destroyed || handle === undefined)
+      return;
+    handle.disableAutoTrailers();
+  }
+
   sendTrailers(headers) {
     if (this.destroyed || this.closed)
       throw new ERR_HTTP2_INVALID_STREAM();
@@ -3187,6 +3201,11 @@ class ServerHttp2Stream extends Http2Stream {
     if (options.waitForTrailers) {
       streamOptions |= STREAM_OPTION_GET_TRAILERS;
       state.flags |= STREAM_FLAGS_HAS_TRAILERS;
+      // Internal mode used by the compat layer: if no trailers have been
+      // registered by the time the final DATA frame is sent, the native
+      // side finishes the stream without calling back into JS at all.
+      if (options[kAutoEmptyTrailers])
+        streamOptions |= STREAM_OPTION_AUTO_EMPTY_TRAILERS;
     }
 
     const {

lib/internal/http2/util.js

@@ -38,6 +38,8 @@ const {
 } = require('internal/errors');
 
 const kAuthority = Symbol('authority');
+const kAutoEmptyTrailers = Symbol('autoEmptyTrailers');
+const kDisableAutoTrailers = Symbol('disableAutoTrailers');
 const kSensitiveHeaders = Symbol('sensitiveHeaders');
 const kStrictSingleValueFields = Symbol('strictSingleValueFields');
 const kSocket = Symbol('socket');
@@ -991,6 +993,8 @@ module.exports = {
   getStreamState,
   isPayloadMeaningless,
   kAuthority,
+  kAutoEmptyTrailers,
+  kDisableAutoTrailers,
   kSensitiveHeaders,
   kStrictSingleValueFields,
   kSocket,

src/node_http2.cc

@@ -2308,6 +2308,9 @@ Http2Stream::Http2Stream(Http2Session* session,
   if (options & STREAM_OPTION_GET_TRAILERS)
     set_has_trailers();
 
+  if (options & STREAM_OPTION_AUTO_EMPTY_TRAILERS)
+    set_auto_empty_trailers();
+
   PushStreamListener(&stream_listener_);
 
   if (options & STREAM_OPTION_EMPTY_PAYLOAD)
@@ -2435,6 +2438,9 @@ int Http2Stream::SubmitResponse(const Http2Headers& headers, int options) {
   if (options & STREAM_OPTION_GET_TRAILERS)
     set_has_trailers();
 
+  if (options & STREAM_OPTION_AUTO_EMPTY_TRAILERS)
+    set_auto_empty_trailers();
+
   if (!is_writable())
     options |= STREAM_OPTION_EMPTY_PAYLOAD;
 
@@ -2468,39 +2474,67 @@ int Http2Stream::SubmitInfo(const Http2Headers& headers) {
 }
 
 void Http2Stream::OnTrailers() {
+  CHECK(!this->is_destroyed());
+  set_has_trailers(false);
+  if (!auto_empty_trailers()) {
+    EmitWantTrailers();
+    return;
+  }
+  // The JS side has not registered any trailers, so the stream can be
+  // finished without calling into JS at all. The empty DATA frame cannot be
+  // submitted synchronously because OnTrailers() runs from inside the data
+  // source read callback for the final DATA frame; defer it to the next
+  // turn of the event loop, just like the JS sendTrailers() path does.
+  Debug(this, "auto-submitting empty trailers");
+  env()->SetImmediate(
+      [self = BaseObjectPtr<Http2Stream>(this)](Environment* env) {
+        if (self->is_destroyed()) return;
+        // Hand control back to the JS side if trailers were registered in
+        // the meantime or if submitting the empty DATA frame failed.
+        if (!self->auto_empty_trailers() || self->SubmitEmptyTrailers() != 0)
+          self->EmitWantTrailers();
+      });
+}
+
+void Http2Stream::EmitWantTrailers() {
   Debug(this, "let javascript know we are ready for trailers");
   CHECK(!this->is_destroyed());
   Isolate* isolate = env()->isolate();
   HandleScope scope(isolate);
   Local<Context> context = env()->context();
   Context::Scope context_scope(context);
-  set_has_trailers(false);
   MakeCallback(env()->http2session_on_stream_trailers_function(), 0, nullptr);
 }
 
-// Submit informational headers for a stream.
+// Sending an empty trailers frame poses problems in Safari, Edge & IE.
+// Instead we can just send an empty data frame with NGHTTP2_FLAG_END_STREAM
+// to indicate that the stream is ready to be closed.
+int Http2Stream::SubmitEmptyTrailers() {
+  CHECK(!this->is_destroyed());
+  Http2Scope h2scope(this);
+  Debug(this, "sending empty trailers");
+  Http2Stream::Provider::Stream prov(this, 0);
+  int ret = nghttp2_submit_data(
+      session_->session(),
+      NGHTTP2_FLAG_END_STREAM,
+      id_,
+      *prov);
+  CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
+  return ret;
+}
+
+// Submit trailing headers for a stream.
 int Http2Stream::SubmitTrailers(const Http2Headers& headers) {
   CHECK(!this->is_destroyed());
+  if (headers.length() == 0)
+    return SubmitEmptyTrailers();
   Http2Scope h2scope(this);
   Debug(this, "sending %d trailers", headers.length());
-  int ret;
-  // Sending an empty trailers frame poses problems in Safari, Edge & IE.
-  // Instead we can just send an empty data frame with NGHTTP2_FLAG_END_STREAM
-  // to indicate that the stream is ready to be closed.
-  if (headers.length() == 0) {
-    Http2Stream::Provider::Stream prov(this, 0);
-    ret = nghttp2_submit_data(
-        session_->session(),
-        NGHTTP2_FLAG_END_STREAM,
-        id_,
-        *prov);
-  } else {
-    ret = nghttp2_submit_trailer(
-        session_->session(),
-        id_,
-        headers.data(),
-        headers.length());
-  }
+  int ret = nghttp2_submit_trailer(
+      session_->session(),
+      id_,
+      headers.data(),
+      headers.length());
   CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
   return ret;
 }
@@ -3110,6 +3144,15 @@ void Http2Stream::Trailers(const FunctionCallbackInfo<Value>& args) {
       stream->SubmitTrailers(Http2Headers(env, headers)));
 }
 
+// Called by the JS layer when trailers are registered after the response
+// headers were already submitted with STREAM_OPTION_AUTO_EMPTY_TRAILERS set,
+// so that the trailers are handed back to JS instead of being auto-emptied.
+void Http2Stream::DisableAutoTrailers(const FunctionCallbackInfo<Value>& args) {
+  Http2Stream* stream;
+  ASSIGN_OR_RETURN_UNWRAP(&stream, args.This());
+  stream->set_auto_empty_trailers(false);
+}
+
 // Grab the numeric id of the Http2Stream
 void Http2Stream::GetID(const FunctionCallbackInfo<Value>& args) {
   Http2Stream* stream;
@@ -3547,6 +3590,10 @@ void Initialize(Local<Object> target,
   SetProtoMethod(isolate, stream, "pushPromise", Http2Stream::PushPromise);
   SetProtoMethod(isolate, stream, "info", Http2Stream::Info);
   SetProtoMethod(isolate, stream, "trailers", Http2Stream::Trailers);
+  SetProtoMethod(isolate,
+                 stream,
+                 "disableAutoTrailers",
+                 Http2Stream::DisableAutoTrailers);
   SetProtoMethod(isolate, stream, "respond", Http2Stream::Respond);
   SetProtoMethod(isolate, stream, "rstStream", Http2Stream::RstStream);
   SetProtoMethod(isolate, stream, "refreshState", Http2Stream::RefreshState);

src/node_http2.h

@@ -57,6 +57,10 @@ constexpr int STREAM_OPTION_EMPTY_PAYLOAD = 0x1;
 // Stream might have trailing headers
 constexpr int STREAM_OPTION_GET_TRAILERS = 0x2;
 
+// Stream may finish with an empty DATA frame carrying END_STREAM without
+// calling back into JS, unless trailers are registered before then
+constexpr int STREAM_OPTION_AUTO_EMPTY_TRAILERS = 0x4;
+
 // Http2Stream internal states
 constexpr int kStreamStateNone = 0x0;
 constexpr int kStreamStateShut = 0x1;
@@ -66,6 +70,7 @@ constexpr int kStreamStateClosed = 0x8;
 constexpr int kStreamStateDestroyed = 0x10;
 constexpr int kStreamStateTrailers = 0x20;
 constexpr int kStreamStatePeerReset = 0x40;
+constexpr int kStreamStateAutoEmptyTrailers = 0x80;
 
 // Http2Session internal states
 constexpr int kSessionStateNone = 0x0;
@@ -310,7 +315,9 @@ class Http2Stream : public AsyncWrap,
 
   // Submit trailing headers for this stream
   int SubmitTrailers(const Http2Headers& headers);
+  int SubmitEmptyTrailers();
   void OnTrailers();
+  void EmitWantTrailers();
 
   // Submit a PRIORITY frame for this stream
   int SubmitPriority(const Http2Priority& priority, bool silent = false);
@@ -368,6 +375,17 @@ class Http2Stream : public AsyncWrap,
       flags_ &= ~kStreamStateTrailers;
   }
 
+  bool auto_empty_trailers() const {
+    return flags_ & kStreamStateAutoEmptyTrailers;
+  }
+
+  void set_auto_empty_trailers(bool on = true) {
+    if (on)
+      flags_ |= kStreamStateAutoEmptyTrailers;
+    else
+      flags_ &= ~kStreamStateAutoEmptyTrailers;
+  }
+
   void set_closed() {
     flags_ |= kStreamStateClosed;
   }
@@ -463,6 +481,8 @@ class Http2Stream : public AsyncWrap,
   static void RefreshState(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Info(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Trailers(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void DisableAutoTrailers(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Respond(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void RstStream(const v8::FunctionCallbackInfo<v8::Value>& args);
 
@@ -1119,7 +1139,8 @@ class Origins {
   V(NGHTTP2_ERR_STREAM_CLOSED)                                                 \
   V(NGHTTP2_ERR_NOMEM)                                                         \
   V(STREAM_OPTION_EMPTY_PAYLOAD)                                               \
-  V(STREAM_OPTION_GET_TRAILERS)
+  V(STREAM_OPTION_GET_TRAILERS)                                                \
+  V(STREAM_OPTION_AUTO_EMPTY_TRAILERS)
 
 #define HTTP2_ERROR_CODES(V)                                                   \
   V(NGHTTP2_NO_ERROR)                                                          \

test/parallel/test-http2-compat-serverresponse-trailers-streaming.js

@@ -0,0 +1,70 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const h2 = require('http2');
+
+// Regression test for the auto-empty-trailers optimization: when the
+// response headers are flushed before the response is ended (streaming
+// mode), trailers registered afterwards must still be sent, and responses
+// that never register trailers must complete normally.
+
+const server = h2.createServer();
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  server.on('request', (request, response) => {
+    if (request.url === '/trailers') {
+      response.writeHead(200);
+      response.write('hello');
+      // Trailers registered after the headers were already flushed.
+      response.setTrailer('x-checksum', 'abc');
+      response.addTrailers({ 'x-count': 2 });
+      response.end('world');
+    } else {
+      response.writeHead(200);
+      response.write('no');
+      response.end('trailers');
+    }
+  });
+
+  const client = h2.connect(`http://localhost:${port}`);
+
+  {
+    const request = client.request({ ':path': '/trailers' });
+    let body = '';
+    request.setEncoding('utf8');
+    request.on('data', (chunk) => body += chunk);
+    request.on('trailers', common.mustCall((trailers) => {
+      assert.strictEqual(trailers['x-checksum'], 'abc');
+      assert.strictEqual(trailers['x-count'], '2');
+    }));
+    request.on('end', common.mustCall(() => {
+      assert.strictEqual(body, 'helloworld');
+      maybeClose();
+    }));
+    request.end();
+  }
+
+  {
+    const request = client.request({ ':path': '/plain' });
+    let body = '';
+    request.setEncoding('utf8');
+    request.on('data', (chunk) => body += chunk);
+    request.on('trailers', common.mustNotCall());
+    request.on('end', common.mustCall(() => {
+      assert.strictEqual(body, 'notrailers');
+      maybeClose();
+    }));
+    request.end();
+  }
+
+  let remaining = 2;
+  function maybeClose() {
+    if (--remaining === 0) {
+      client.close();
+      server.close();
+    }
+  }
+}));