From 7b937eb313e60e9e1fd8cfcf1810fc4fc5b4d966 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Thu, 21 Feb 2019 23:54:05 +0100
Subject: [PATCH 1/2] src: do not access Environment-owned handles after
 cleanup

Do not access handles that have already begun to be closed
or are closed.
---
 src/env.cc | 12 +++++++++++-
 src/env.h  |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/env.cc b/src/env.cc
index 01aee464406628..f2319802d088be 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -394,7 +394,11 @@ void Environment::RegisterHandleCleanups() {
                                         void* arg) {
     handle->data = env;
 
-    env->CloseHandle(handle, [](uv_handle_t* handle) {});
+    env->CloseHandle(handle, [](uv_handle_t* handle) {
+#ifdef DEBUG
+      memset(handle, 0xab, uv_handle_size(handle->type));
+#endif
+    });
   };
 
   RegisterHandleCleanup(
@@ -508,6 +512,7 @@ void Environment::PrintSyncTrace() const {
 }
 
 void Environment::RunCleanup() {
+  started_cleanup_ = true;
   TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment),
                               "RunCleanup", this);
   CleanupHandles();
@@ -656,10 +661,13 @@ void Environment::RunAndClearNativeImmediates() {
 
 
 void Environment::ScheduleTimer(int64_t duration_ms) {
+  if (started_cleanup_) return;
   uv_timer_start(timer_handle(), RunTimers, duration_ms, 0);
 }
 
 void Environment::ToggleTimerRef(bool ref) {
+  if (started_cleanup_) return;
+
   if (ref) {
     uv_ref(reinterpret_cast<uv_handle_t*>(timer_handle()));
   } else {
@@ -759,6 +767,8 @@ void Environment::CheckImmediate(uv_check_t* handle) {
 }
 
 void Environment::ToggleImmediateRef(bool ref) {
+  if (started_cleanup_) return;
+
   if (ref) {
     // Idle handle is needed only to stop the event loop from blocking in poll.
     uv_idle_start(immediate_idle_handle(), [](uv_idle_t*){ });
diff --git a/src/env.h b/src/env.h
index 5f578dd54a9a7f..1f8bf0f44a7bf5 100644
--- a/src/env.h
+++ b/src/env.h
@@ -1126,6 +1126,7 @@ class Environment {
                      CleanupHookCallback::Hash,
                      CleanupHookCallback::Equal> cleanup_hooks_;
   uint64_t cleanup_hook_counter_ = 0;
+  bool started_cleanup_ = false;
 
   static void EnvPromiseHook(v8::PromiseHookType type,
                              v8::Local<v8::Promise> promise,

From c43d20bc84212db6daf54167428d91705e2f3a13 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Thu, 21 Feb 2019 23:12:01 +0100
Subject: [PATCH 2/2] src: clean up `StreamPipe` in destructor

In the presence of Workers, it is not safe to assume that
`StreamPipe::Unpipe()` has been called at the time when the object
is destroyed.

Instead, clean up when the destructor is called.
---
 src/stream_pipe.cc                            |  2 +-
 ...orker-terminate-http2-respond-with-file.js | 39 +++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-worker-terminate-http2-respond-with-file.js

diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc
index 19d732d6592aaa..c0adcf380f0771 100644
--- a/src/stream_pipe.cc
+++ b/src/stream_pipe.cc
@@ -42,7 +42,7 @@ StreamPipe::StreamPipe(StreamBase* source,
 }
 
 StreamPipe::~StreamPipe() {
-  CHECK(is_closed_);
+  Unpipe();
 }
 
 StreamBase* StreamPipe::source() {
diff --git a/test/parallel/test-worker-terminate-http2-respond-with-file.js b/test/parallel/test-worker-terminate-http2-respond-with-file.js
new file mode 100644
index 00000000000000..80b58ddfc5bac0
--- /dev/null
+++ b/test/parallel/test-worker-terminate-http2-respond-with-file.js
@@ -0,0 +1,39 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+const makeDuplexPair = require('../common/duplexpair');
+const { Worker, isMainThread } = require('worker_threads');
+
+// This is a variant of test-http2-generic-streams-sendfile for checking
+// that Workers can be terminated during a .respondWithFile() operation.
+
+if (isMainThread) {
+  return new Worker(__filename);
+}
+
+{
+  const server = http2.createServer();
+  server.on('stream', common.mustCall((stream, headers) => {
+    stream.respondWithFile(process.execPath);  // Use a large-ish file.
+  }));
+
+  const { clientSide, serverSide } = makeDuplexPair();
+  server.emit('connection', serverSide);
+
+  const client = http2.connect('http://localhost:80', {
+    createConnection: common.mustCall(() => clientSide)
+  });
+
+  const req = client.request();
+
+  req.on('response', common.mustCall((headers) => {
+    assert.strictEqual(headers[':status'], 200);
+  }));
+
+  req.on('data', common.mustCall(process.exit));
+  req.on('end', common.mustNotCall());
+  req.end();
+}