From 9628ee76fc0390d4a095e84dba8eaaa78ce36ffb Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Tue, 13 Aug 2024 00:24:23 -0700 Subject: [PATCH] windows: fix fs-promises-writeFile-async-iterator.test.ts [v2] (#13164) Co-authored-by: Jarred Sumner --- src/bun.js/webcore/blob.zig | 1 + src/bun.js/webcore/streams.zig | 5 ++-- src/io/PipeWriter.zig | 6 +++++ test/js/bun/util/filesink.test.ts | 42 ++++++++++++++++++++++++++++++- 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/bun.js/webcore/blob.zig b/src/bun.js/webcore/blob.zig index 7185f80849..71980e0ce2 100644 --- a/src/bun.js/webcore/blob.zig +++ b/src/bun.js/webcore/blob.zig @@ -3421,6 +3421,7 @@ pub const Blob = struct { }; }; var sink = JSC.WebCore.FileSink.init(fd, this.globalThis.bunVM().eventLoop()); + sink.writer.owns_fd = pathlike != .fd; if (is_stdout_or_stderr) { switch (sink.writer.startSync(fd, false)) { diff --git a/src/bun.js/webcore/streams.zig b/src/bun.js/webcore/streams.zig index f00258029e..04c42380a0 100644 --- a/src/bun.js/webcore/streams.zig +++ b/src/bun.js/webcore/streams.zig @@ -3036,13 +3036,13 @@ pub const FileSink = struct { // if we are not done yet and has pending data we just wait so we do not runPending twice if (status == .pending and has_pending_data) { if (this.pending.state == .pending) { - this.pending.consumed += @truncate(amount); + this.pending.consumed = @truncate(amount); } return; } if (this.pending.state == .pending) { - this.pending.consumed += @truncate(amount); + this.pending.consumed = @truncate(amount); // when "done" is true, we will never receive more data. if (this.done or status == .end_of_file) { @@ -3087,6 +3087,7 @@ pub const FileSink = struct { this.signal.ready(null, null); } + pub fn onClose(this: *FileSink) void { log("onClose()", .{}); this.signal.close(null); diff --git a/src/io/PipeWriter.zig b/src/io/PipeWriter.zig index 54f4f19e46..33a7767505 100644 --- a/src/io/PipeWriter.zig +++ b/src/io/PipeWriter.zig @@ -891,6 +891,7 @@ pub fn WindowsBufferedWriter( ) type { return struct { source: ?Source = null, + owns_fd: bool = true, parent: *Parent = undefined, is_done: bool = false, // we use only one write_req, any queued data in outgoing will be flushed after this ends @@ -1003,6 +1004,7 @@ pub fn WindowsBufferedWriter( this.is_done = true; if (this.pending_payload_size == 0) { // will auto close when pending stuff get written + if (!this.owns_fd) return; this.close(); } } @@ -1131,6 +1133,9 @@ pub fn WindowsStreamingWriter( ) type { return struct { source: ?Source = null, + /// if the source of this writer is a file descriptor, calling end() will not close it. + /// if it is a path, then we claim ownership and the backing fd will be closed by end(). + owns_fd: bool = true, parent: *Parent = undefined, is_done: bool = false, // we use only one write_req, any queued data in outgoing will be flushed after this ends @@ -1382,6 +1387,7 @@ pub fn WindowsStreamingWriter( this.closed_without_reporting = false; // if we are done we can call close if not we wait all the data to be flushed if (this.isDone()) { + if (!this.owns_fd) return; this.close(); } } diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index b9bab1b8ca..d0cba5af53 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { fileDescriptorLeakChecker, getMaxFD, isWindows, tmpdirSync } from "harness"; +import { fileDescriptorLeakChecker, isWindows, tmpdirSync } from "harness"; import { mkfifo } from "mkfifo"; import { join } from "node:path"; @@ -165,3 +165,43 @@ describe("FileSink", () => { }); } }); + +import fs from "node:fs"; +import util from "node:util"; +import path from "node:path"; + +it("end doesn't close when backed by a file descriptor", async () => { + using _ = fileDescriptorLeakChecker(); + const x = tmpdirSync(); + const fd = await util.promisify(fs.open)(path.join(x, "test.txt"), "w"); + const chunk = Buffer.from("1 Hello, world!"); + const file = Bun.file(fd); + const writer = file.writer(); + const written = await writer.write(chunk); + await writer.end(); + await util.promisify(fs.ftruncate)(fd, written); + await util.promisify(fs.close)(fd); +}); + +it("end does close when not backed by a file descriptor", async () => { + using _ = fileDescriptorLeakChecker(); + const x = tmpdirSync(); + const file = Bun.file(path.join(x, "test.txt")); + const writer = file.writer(); + await writer.write(Buffer.from("1 Hello, world!")); + await writer.end(); + await Bun.sleep(10); // For the file descriptor leak checker. +}); + +it("write result is not cummulative", async () => { + using _ = fileDescriptorLeakChecker(); + const x = tmpdirSync(); + const fd = await util.promisify(fs.open)(path.join(x, "test.txt"), "w"); + const file = Bun.file(fd); + const writer = file.writer(); + expect(await writer.write("1 ")).toBe(2); + expect(await writer.write("Hello, ")).toBe(7); + expect(await writer.write("world!")).toBe(6); + await writer.end(); + await util.promisify(fs.close)(fd); +});