From 5a6ef045aaa0de98ac529e0b898955c017ff7bab Mon Sep 17 00:00:00 2001
From: Lukas Kastern <lukaskastern@web.de>
Date: Wed, 17 Jan 2024 00:26:23 +0100
Subject: [PATCH] fix: Hang when recursively logging via
 ConsoleObject.messageWithTypeAndLevel  (#8213)

* Avoid deadlock in messageWithTypeAndLevel by adding a recursion counter

* Add test for recursive logs

* Do not rely on output of console-recursive.test.js

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
---
 src/bun.js/ConsoleObject.zig                  | 28 ++++++++++++++++---
 test/js/web/console/console-recursive.js      | 16 +++++++++++
 test/js/web/console/console-recursive.test.ts | 26 +++++++++++++++++
 3 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100644 test/js/web/console/console-recursive.js
 create mode 100644 test/js/web/console/console-recursive.test.ts

diff --git a/src/bun.js/ConsoleObject.zig b/src/bun.js/ConsoleObject.zig
index 351c5d0bd1..1369d0bf98 100644
--- a/src/bun.js/ConsoleObject.zig
+++ b/src/bun.js/ConsoleObject.zig
@@ -72,6 +72,9 @@ pub const MessageType = enum(u32) {
 var stderr_mutex: bun.Lock = bun.Lock.init();
 var stdout_mutex: bun.Lock = bun.Lock.init();
 
+threadlocal var stderr_lock_count: u16 = 0;
+threadlocal var stdout_lock_count: u16 = 0;
+
 /// https://console.spec.whatwg.org/#formatter
 pub fn messageWithTypeAndLevel(
     //console_: ConsoleObject.Type,
@@ -92,15 +95,32 @@ pub fn messageWithTypeAndLevel(
     // Lock/unlock a mutex incase two JS threads are console.log'ing at the same time
     // We do this the slightly annoying way to avoid assigning a pointer
     if (level == .Warning or level == .Error or message_type == .Assert) {
-        stderr_mutex.lock();
+        if (stderr_lock_count == 0) {
+            stderr_mutex.lock();
+        }
+
+        stderr_lock_count += 1;
     } else {
-        stdout_mutex.lock();
+        if (stdout_lock_count == 0) {
+            stdout_mutex.lock();
+        }
+
+        stdout_lock_count += 1;
     }
+
     defer {
         if (level == .Warning or level == .Error or message_type == .Assert) {
-            stderr_mutex.unlock();
+            stderr_lock_count -= 1;
+
+            if (stderr_lock_count == 0) {
+                stderr_mutex.unlock();
+            }
         } else {
-            stdout_mutex.unlock();
+            stdout_lock_count -= 1;
+
+            if (stdout_lock_count == 0) {
+                stdout_mutex.unlock();
+            }
         }
     }
 
diff --git a/test/js/web/console/console-recursive.js b/test/js/web/console/console-recursive.js
new file mode 100644
index 0000000000..d62d345bcd
--- /dev/null
+++ b/test/js/web/console/console-recursive.js
@@ -0,0 +1,16 @@
+// We use a relatively random string for the stderr flag.
+// Just to avoid matching against something in the executable path.
+const use_err = process.argv.includes("print_to_stderr_skmxctoznf");
+
+const log_method = use_err ? console.error : console.log;
+
+class MyMap extends Map {
+  get size() {
+    log_method("inside size");
+    return 0;
+  }
+}
+
+const map = new MyMap();
+
+log_method("map:", map);
diff --git a/test/js/web/console/console-recursive.test.ts b/test/js/web/console/console-recursive.test.ts
new file mode 100644
index 0000000000..fed98e1bb8
--- /dev/null
+++ b/test/js/web/console/console-recursive.test.ts
@@ -0,0 +1,26 @@
+// @known-failing-on-windows: 1 failing
+import { spawn } from "bun";
+import { expect, it } from "bun:test";
+import { bunExe, bunEnv } from "harness";
+
+it("should not hang when logging to stdout recursively", async () => {
+  const { exited } = spawn({
+    cmd: [bunExe(), import.meta.dir + "/console-recursive.js"],
+    stdin: null,
+    stdout: "pipe",
+    stderr: "pipe",
+    env: bunEnv,
+  });
+  expect(await exited).toBe(0);
+});
+
+it("should not hang when logging to stderr recursively", async () => {
+  const { exited } = spawn({
+    cmd: [bunExe(), import.meta.dir + "/console-recursive.js", "print_to_stderr_skmxctoznf"],
+    stdin: null,
+    stdout: "pipe",
+    stderr: "pipe",
+    env: bunEnv,
+  });
+  expect(await exited).toBe(0);
+});