From 5b1d00c595da403728a764c437273bd5618a7bb4 Mon Sep 17 00:00:00 2001
From: Philip Russell <argosphil@murena.io>
Date: Tue, 6 Feb 2024 14:00:58 +0000
Subject: [PATCH 1/3] Fix FIFO data loss (#8695)

Under certain circumstances, we would read data from FIFOs without a
sensible place to put it, leading to dropped chunks in async iteration
over Bun.stdin.stream().

Fixes #8695.
---
 src/bun.js/webcore/streams.zig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/bun.js/webcore/streams.zig b/src/bun.js/webcore/streams.zig
index a1db89e25a9fe3..b0b16649668158 100644
--- a/src/bun.js/webcore/streams.zig
+++ b/src/bun.js/webcore/streams.zig
@@ -4320,6 +4320,8 @@ pub fn NewFIFO(comptime EventLoop: JSC.EventLoopKind) type {
                 }
             }
 
+            if (this.pending.state != .pending) return;
+
             const read_result = this.read(
                 this.buf,
                 // On Linux, we end up calling ioctl() twice if we don't do this

From 4f5a10ff702256e1446d66f884728c2591639d69 Mon Sep 17 00:00:00 2001
From: Philip Russell <argosphil@murena.io>
Date: Wed, 7 Feb 2024 14:27:36 +0000
Subject: [PATCH 2/3] Test for #8695

This test seems to fail reliably with the current bun release. Uses
stderr rather than stdout because bun-debug currently outputs debug
information to stdout and I didn't want to filter it.

The test simply counts to 20, taking 100 ms for each write, while the
reader consumes chunks and takes one second to asynchronously process
each one.

Due to the timing-sensitive nature of the tested code, it does take 3
seconds to run, which I think is acceptable.

Output with current bun release:

490 |   let text = "";
491 |   for await (const chunk of producer.stderr) {
492 |     text += [...chunk].map(x => String.fromCharCode(x)).join("");
493 |     await new Promise(r => setTimeout(r, 1000));
494 |   }
495 |   expect(text).toBe("0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14\n15\n16\n17\n18\n19\n20\n");
        ^
error: expect(received).toBe(expected)

Expected: "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14\n15\n16\n17\n18\n19\n20\n"
Received: "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n11\n12\n13\n14\n15\n16\n17\n18\n19\n"

      at .../bun/test/js/bun/io/bun-write.test.js:495:3
(fail) timed output should work [3022.97ms]
---
 test/js/bun/io/bun-write.test.js      | 15 +++++++++++++++
 test/js/bun/io/timed-stderr-output.js |  4 ++++
 2 files changed, 19 insertions(+)
 create mode 100644 test/js/bun/io/timed-stderr-output.js

diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js
index 33c5c61d415c39..60534849e6c6db 100644
--- a/test/js/bun/io/bun-write.test.js
+++ b/test/js/bun/io/bun-write.test.js
@@ -479,3 +479,18 @@ describe("ENOENT", () => {
     });
   });
 });
+
+it("timed output should work", async () => {
+  const producer_file = path.join(import.meta.dir, "timed-stderr-output.js");
+
+  const producer = Bun.spawn([bunExe(), "run", producer_file], {
+    stderr: "pipe",
+  });
+
+  let text = "";
+  for await (const chunk of producer.stderr) {
+    text += [...chunk].map(x => String.fromCharCode(x)).join("");
+    await new Promise(r => setTimeout(r, 1000));
+  }
+  expect(text).toBe("0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14\n15\n16\n17\n18\n19\n20\n");
+});
diff --git a/test/js/bun/io/timed-stderr-output.js b/test/js/bun/io/timed-stderr-output.js
new file mode 100644
index 00000000000000..be25f390327cf6
--- /dev/null
+++ b/test/js/bun/io/timed-stderr-output.js
@@ -0,0 +1,4 @@
+for (let i = 0; i <= 20; i++) {
+  console.error(i);
+  await new Promise(r => setTimeout(r, 100));
+}

From bfef103e587c0a1a6c9dc8ac54d74386a172754f Mon Sep 17 00:00:00 2001
From: Philip Russell <argosphil@murena.io>
Date: Wed, 7 Feb 2024 20:59:11 +0000
Subject: [PATCH 3/3] avoid console.error, use Bun.write(Bun.stderr, ...)
 instead

This should fix failures of the new test in CI because of escape
sequences injected by console.error.

Once bun-debug is fixed to print debug output to stderr, this test
will need adjusting.
---
 test/js/bun/io/timed-stderr-output.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/js/bun/io/timed-stderr-output.js b/test/js/bun/io/timed-stderr-output.js
index be25f390327cf6..42fc5becde3046 100644
--- a/test/js/bun/io/timed-stderr-output.js
+++ b/test/js/bun/io/timed-stderr-output.js
@@ -1,4 +1,4 @@
 for (let i = 0; i <= 20; i++) {
-  console.error(i);
+  await Bun.write(Bun.stderr, i + "\n");
   await new Promise(r => setTimeout(r, 100));
 }