From aeb3ab8e42eaf93db04b77e9698379ce2fec9610 Mon Sep 17 00:00:00 2001
From: Ryan Fitzgerald <rwfitzge@gmail.com>
Date: Fri, 28 Oct 2022 14:56:09 -0700
Subject: [PATCH 1/2] Fix panic triggered by boxed Channel instances

This fixes a bug that can be triggered when there's a JsBox holding an
instance of Channel at the moment the VM stops (e.g., when quitting an
Electron app). The Channel's `drop` implementation calls `self.send`,
which panics with a SendError because we're already in the middle of
tearing everything down.
---
 crates/neon/src/event/channel.rs |  4 +++-
 test/napi/lib/workers.js         |  3 +++
 test/napi/src/js/workers.rs      | 17 +++++++++++++++++
 test/napi/src/lib.rs             |  1 +
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/crates/neon/src/event/channel.rs b/crates/neon/src/event/channel.rs
index 090b4c135..e82952ff5 100644
--- a/crates/neon/src/event/channel.rs
+++ b/crates/neon/src/event/channel.rs
@@ -249,7 +249,9 @@ impl Drop for Channel {
         // UV thread if strong reference count goes to 0.
         let state = Arc::clone(&self.state);
 
-        self.send(move |mut cx| {
+        // Use `try_send` instead of `send` because, if `send` fails, it means we're shutting down
+        // the VM and don't need to worry about cleanup.
+        let _ = self.try_send(move |mut cx| {
             state.unref(&mut cx);
             Ok(())
         });
diff --git a/test/napi/lib/workers.js b/test/napi/lib/workers.js
index ab2c9c61e..dc0daed3a 100644
--- a/test/napi/lib/workers.js
+++ b/test/napi/lib/workers.js
@@ -15,6 +15,9 @@ if (!isMainThread) {
   // succeed even if the presence of a bug.
   addon.reject_after(new Error("Oh, no!"), 200).catch(() => {});
 
+  // Reproduce another shutdown bug; this one isn't timing-dependent.
+  let boxed_channels = addon.box_channels();
+
   addon.get_or_init_thread_id(threadId);
   parentPort.once("message", (message) => {
     try {
diff --git a/test/napi/src/js/workers.rs b/test/napi/src/js/workers.rs
index d95c3ff75..7a49c76a5 100644
--- a/test/napi/src/js/workers.rs
+++ b/test/napi/src/js/workers.rs
@@ -108,3 +108,20 @@ pub fn reject_after(mut cx: FunctionContext) -> JsResult<JsPromise> {
 
     Ok(promise)
 }
+
+#[allow(dead_code)]
+pub struct Channels {
+    channel_1: Channel,
+    channel_2: Channel,
+}
+impl Finalize for Channels {}
+
+pub fn box_channels(mut cx: FunctionContext) -> JsResult<JsBox<Channels>> {
+    let channel_1 = cx.channel();
+    let channel_2 = cx.channel();
+
+    Ok(cx.boxed(Channels {
+        channel_1,
+        channel_2,
+    }))
+}
diff --git a/test/napi/src/lib.rs b/test/napi/src/lib.rs
index 58f1a1ba2..4739a889e 100644
--- a/test/napi/src/lib.rs
+++ b/test/napi/src/lib.rs
@@ -384,6 +384,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
     cx.export_function("stash_global_object", js::workers::stash_global_object)?;
     cx.export_function("unstash_global_object", js::workers::unstash_global_object)?;
     cx.export_function("reject_after", js::workers::reject_after)?;
+    cx.export_function("box_channels", js::workers::box_channels)?;
 
     // Futures
     cx.export_function("lazy_async_add", js::futures::lazy_async_add)?;

From c0330a3449e2fbb49bb6798bd3c20302902a577f Mon Sep 17 00:00:00 2001
From: Ryan Fitzgerald <rwfitzge@gmail.com>
Date: Fri, 4 Nov 2022 16:16:29 -0700
Subject: [PATCH 2/2] Wait for Worker in test to initialize before terminating

This fixes a sporadic panic caused by a race condition between a Worker
loading our test-specific Rust extension and the worker being shut down.
This shouldn't panic in principle, but fixing it would require
introducing a leak (see 2bf1525fe8 for the change we could have made and
decided not to).
---
 test/napi/lib/workers.js | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/test/napi/lib/workers.js b/test/napi/lib/workers.js
index dc0daed3a..78eade154 100644
--- a/test/napi/lib/workers.js
+++ b/test/napi/lib/workers.js
@@ -4,6 +4,7 @@ const {
   isMainThread,
   parentPort,
   threadId,
+  workerData,
 } = require("worker_threads");
 
 const addon = require("..");
@@ -19,6 +20,7 @@ if (!isMainThread) {
   let boxed_channels = addon.box_channels();
 
   addon.get_or_init_thread_id(threadId);
+
   parentPort.once("message", (message) => {
     try {
       switch (message) {
@@ -47,6 +49,10 @@ if (!isMainThread) {
     }
   });
 
+  if (workerData === "notify_when_startup_complete") {
+    parentPort.postMessage("startup_complete");
+  }
+
   return;
 }
 
@@ -197,9 +203,13 @@ describe("Instance-local storage", () => {
   });
 
   it("should be able to exit a worker without a crash", (cb) => {
-    const worker = new Worker(__filename);
+    const worker = new Worker(__filename, {
+      workerData: "notify_when_startup_complete",
+    });
 
-    setTimeout(() => worker.terminate(), 50);
-    setTimeout(cb, 100);
+    worker.once("message", async () => {
+      await worker.terminate();
+      cb();
+    });
   });
 });