http2: skip creating native ShutdownWrap

addaleax · MylesBorins · commit 52504fb91e68 · 2020-01-16T01:39:12.000-05:00
`ShutdownWrap` instances are being used to carry context between the start and the asynchronous end of shutting down the writable side of a `StreamBase`. HTTP/2 streams always perform this action synchronously, so no such object is necessary. PR-URL: #31283 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -1959,6 +1959,12 @@ void Http2Stream::Close(int32_t code) {
   Debug(this, "closed with code %d", code);
 }
 
+ShutdownWrap* Http2Stream::CreateShutdownWrap(v8::Local<v8::Object> object) {
+  // DoShutdown() always finishes synchronously, so there's no need to create
+  // a structure to store asynchronous context.
+  return nullptr;
+}
+
 int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
   if (IsDestroyed())
     return UV_EPIPE;
diff --git a/src/node_http2.h b/src/node_http2.h
@@ -470,6 +470,7 @@ class Http2Stream : public AsyncWrap,
   int ReadStop() override;
 
   // Required for StreamBase
+  ShutdownWrap* CreateShutdownWrap(v8::Local<v8::Object> object) override;
   int DoShutdown(ShutdownWrap* req_wrap) override;
 
   bool HasWantsWrite() const override { return true; }
diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h
@@ -162,7 +162,7 @@ inline int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {
   ShutdownWrap* req_wrap = CreateShutdownWrap(req_wrap_obj);
   int err = DoShutdown(req_wrap);
 
-  if (err != 0) {
+  if (err != 0 && req_wrap != nullptr) {
     req_wrap->Dispose();
   }
 

Original file line number	Diff line number	Diff line change
`@@ -162,7 +162,7 @@ inline int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {`
`162`	`162`	`ShutdownWrap* req_wrap = CreateShutdownWrap(req_wrap_obj);`
`163`	`163`	`int err = DoShutdown(req_wrap);`
`164`	`164`
`165`		`- if (err != 0) {`
	`165`	`+ if (err != 0 && req_wrap != nullptr) {`
`166`	`166`	`req_wrap->Dispose();`
`167`	`167`	`}`
`168`	`168`