bug compatible with `stdin.on("readable")` (#1626)

3 files changed, 95 insertions, 49 deletions

diff --git a/src/bun.js/builtins/js/ProcessObjectInternals.js b/src/bun.js/builtins/js/ProcessObjectInternals.js
index e07d3389b..792f59ec3 100644
--- a/src/bun.js/builtins/js/ProcessObjectInternals.js
+++ b/src/bun.js/builtins/js/ProcessObjectInternals.js
@@ -435,7 +435,7 @@ function getStdioWriteStream(fd_, rawRequire) {
   return new FastStdioWriteStream(fd_);
 }
 
-function getStdinStream(fd, rawRequire, Bun) {
+function getStdinStream(fd_, rawRequire, Bun) {
   var module = { path: "node:process", require: rawRequire };
   var require = (path) => module.require(path);
 
@@ -449,6 +449,7 @@ function getStdinStream(fd, rawRequire, Bun) {
     #writeStream;
 
     #readable = true;
+    #unrefOnRead = false;
     #writable = true;
 
     #onFinish;
@@ -456,7 +457,7 @@ function getStdinStream(fd, rawRequire, Bun) {
     #onDrain;
 
     get isTTY() {
-      return require("tty").isatty(fd);
+      return require("tty").isatty(fd_);
     }
 
     get fd() {
@@ -509,20 +510,38 @@ function getStdinStream(fd, rawRequire, Bun) {
       }
     }
 
+    on(name, callback) {
+      // Streams don't generally required to present any data when only
+      // `readable` events are present, i.e. `readableFlowing === false`
+      //
+      // However, Node.js has a this quirk whereby `process.stdin.read()`
+      // blocks under TTY mode, thus looping `.read()` in this particular
+      // case would not result in truncation.
+      //
+      // Therefore the following hack is only specific to `process.stdin`
+      // and does not apply to the underlying Stream implementation.
+      if (name === "readable") {
+        this.ref();
+        this.#unrefOnRead = true;
+      }
+      return super.on(name, callback);
+    }
+
     pause() {
       this.unref();
       return super.pause();
     }
 
     resume() {
-      this.#reader ??= Bun.stdin.stream().getReader();
       this.ref();
       return super.resume();
     }
 
     ref() {
+      this.#reader ??= Bun.stdin.stream().getReader();
       this.#readRef ??= setInterval(() => {}, 1 << 30);
     }
+
     unref() {
       if (this.#readRef) {
         clearInterval(this.#readRef);
@@ -563,6 +582,10 @@ function getStdinStream(fd, rawRequire, Bun) {
     }
 
     _read(size) {
+      if (this.#unrefOnRead) {
+        this.unref();
+        this.#unrefOnRead = false;
+      }
       this.#readInternal();
     }
 
diff --git a/test/bun.js/child-process-stdio.test.js b/test/bun.js/child-process-stdio.test.js
index 9969473be..a023a6e03 100644
--- a/test/bun.js/child-process-stdio.test.js
+++ b/test/bun.js/child-process-stdio.test.js
@@ -5,53 +5,82 @@ const CHILD_PROCESS_FILE = import.meta.dir + "/spawned-child.js";
 const OUT_FILE = import.meta.dir + "/stdio-test-out.txt";
 
 describe("process.stdout", () => {
-  it("should allow us to write to it", () => {
-    process.stdout.write("Bun is cool\n");
+  it("should allow us to write to it", (done) => {
+    const child = spawn("bun", [CHILD_PROCESS_FILE, "STDOUT"]);
+    child.stdout.setEncoding("utf8");
+    child.stdout.on("data", (data) => {
+      try {
+        expect(data).toBe("stdout_test");
+        done();
+      } catch (err) {
+        done(err);
+      }
+    });
   });
 });
 
 describe("process.stdin", () => {
   it("should allow us to read from stdin in readable mode", (done) => {
+    const input = "hello\n";
     // Child should read from stdin and write it back
     const child = spawn("bun", [CHILD_PROCESS_FILE, "STDIN", "READABLE"]);
+    let data = "";
     child.stdout.setEncoding("utf8");
-    child.stdout.on("data", (data) => {
-      expect(data.trim()).toBe("data: hello");
-      done();
+    child.stdout.on("data", (chunk) => {
+      data += chunk;
+    }).on("end", function() {
+      try {
+        expect(data).toBe(`data: ${input}`);
+        done();
+      } catch (err) {
+        done(err);
+      }
     });
-    child.stdin.write("hello\n");
+    child.stdin.write(input);
     child.stdin.end();
   });
 
   it("should allow us to read from stdin via flowing mode", (done) => {
+    const input = "hello\n";
     // Child should read from stdin and write it back
     const child = spawn("bun", [CHILD_PROCESS_FILE, "STDIN", "FLOWING"]);
+    let data = "";
     child.stdout.setEncoding("utf8");
-    child.stdout.on("data", (data) => {
-      expect(data.trim()).toBe("data: hello");
-      done();
+    child.stdout.on("readable", () => {
+      let chunk;
+      while ((chunk = child.stdout.read()) !== null) {
+        data += chunk;
+      }
+    }).on("end", function() {
+      try {
+        expect(data).toBe(`data: ${input}`);
+        done();
+      } catch (err) {
+        done(err);
+      }
     });
-    child.stdin.write("hello\n");
+    child.stdin.write(input);
     child.stdin.end();
   });
 
   it("should allow us to read > 65kb from stdin", (done) => {
-    // Child should read from stdin and write it back
-    const child = spawn("bun", [CHILD_PROCESS_FILE, "STDIN", "FLOWING"]);
-    child.stdout.setEncoding("utf8");
-
     const numReps = Math.ceil((66 * 1024) / 5);
     const input = "hello".repeat(numReps);
-
+    // Child should read from stdin and write it back
+    const child = spawn("bun", [CHILD_PROCESS_FILE, "STDIN", "FLOWING"]);
     let data = "";
-    child.stdout.on("end", () => {
-      expect(data).toBe(`data: ${input}`);
-      done();
-    });
+    child.stdout.setEncoding("utf8");
     child.stdout.on("readable", () => {
       let chunk;
       while ((chunk = child.stdout.read()) !== null) {
-        data += chunk.trim();
+        data += chunk;
+      }
+    }).on("end", function() {
+      try {
+        expect(data).toBe(`data: ${input}`);
+        done();
+      } catch (err) {
+        done(err);
       }
     });
     child.stdin.write(input);
@@ -65,6 +94,6 @@ describe("process.stdin", () => {
       }/readFileSync.txt`,
       { encoding: "utf8" },
     );
-    expect(result.trim()).toEqual("data: File read successfully");
+    expect(result).toEqual("data: File read successfully");
   });
 });
diff --git a/test/bun.js/spawned-child.js b/test/bun.js/spawned-child.js
index a90dfade2..d39131933 100644
--- a/test/bun.js/spawned-child.js
+++ b/test/bun.js/spawned-child.js
@@ -1,33 +1,27 @@
 const TARGET = process.argv[2];
 const MODE = process.argv[3];
 
-async function main() {
-  if (TARGET === "STDIN") {
-    let data = "";
-    process.stdin.setEncoding("utf8");
-    if (MODE === "READABLE") {
-      process.stdin.on("readable", () => {
-        let chunk;
-        while ((chunk = process.stdin.read()) !== null) {
-          data += chunk;
-        }
-      });
-    } else {
-      process.stdin.on("data", (chunk) => {
+if (TARGET === "STDIN") {
+  let data = "";
+  process.stdin.setEncoding("utf8");
+  if (MODE === "READABLE") {
+    process.stdin.on("readable", () => {
+      let chunk;
+      while ((chunk = process.stdin.read()) !== null) {
         data += chunk;
-      });
-    }
-    process.stdin.on("end", () => {
-      console.log("data:", data);
-      process.exit(0);
+      }
     });
-  } else if (TARGET === "STDOUT") {
-    process.stdout.write("stdout_test");
-  } else if (TARGET === "TIMER") {
-    setTimeout(() => console.log("hello"), 150);
   } else {
-    console.log("unknown target! you messed up...");
+    process.stdin.on("data", (chunk) => {
+      data += chunk;
+    });
   }
+  process.stdin.on("end", () => {
+    process.stdout.write("data: ");
+    process.stdout.write(data);
+  });
+} else if (TARGET === "STDOUT") {
+  process.stdout.write("stdout_test");
+} else {
+  console.log("unknown target! you messed up...");
 }
-
-main();