From 1675349667109d23984fc60fbcc1c4d0a25e70bf Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 26 Mar 2024 13:53:58 -0700 Subject: [PATCH] what happens if nonblocking tty (#9608) Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> --- src/bun.js/bindings/c-bindings.cpp | 49 ++++++++++++++++++++++++- src/bun.js/webcore/streams.zig | 57 +++++++++++++++++++++++++----- src/c.zig | 1 + src/feature_flags.zig | 5 +++ 4 files changed, 102 insertions(+), 10 deletions(-) diff --git a/src/bun.js/bindings/c-bindings.cpp b/src/bun.js/bindings/c-bindings.cpp index bcda72ec28fe1..f77f9e6bb56c8 100644 --- a/src/bun.js/bindings/c-bindings.cpp +++ b/src/bun.js/bindings/c-bindings.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #else #include #include @@ -480,4 +481,50 @@ extern "C" void bun_initialize_process() #endif atexit(Bun__onExit); -} \ No newline at end of file +} + +#if OS(WINDOWS) +extern "C" int32_t open_as_nonblocking_tty(int32_t fd, int32_t mode) +{ + RELEASE_ASSERT_NOT_REACHED(); +} + +#else + +static bool can_open_as_nonblocking_tty(int32_t fd) +{ + int result; +#if OS(LINUX) || OS(FreeBSD) + int dummy = 0; + + result = ioctl(fd, TIOCGPTN, &dummy) != 0; +#elif OS(DARWIN) + + char dummy[256]; + + result = ioctl(fd, TIOCPTYGNAME, &dummy) != 0; + +#else + +#error "TODO" + +#endif + + return result; +} + +extern "C" int32_t open_as_nonblocking_tty(int32_t fd, int32_t mode) +{ + if (!can_open_as_nonblocking_tty(fd)) { + return -1; + } + + char pathbuf[PATH_MAX + 1]; + if (ttyname_r(fd, pathbuf, sizeof(pathbuf)) != 0) { + return -1; + } + + return open(pathbuf, mode | O_NONBLOCK | O_NOCTTY | O_CLOEXEC); +} + +#endif \ No newline at end of file diff --git a/src/bun.js/webcore/streams.zig b/src/bun.js/webcore/streams.zig index 158b7a752de79..d37c4729b629e 100644 --- a/src/bun.js/webcore/streams.zig +++ b/src/bun.js/webcore/streams.zig @@ -3039,9 +3039,24 @@ pub const FileSink = struct { pub fn setup(this: *FileSink, options: *const StreamStart.FileSinkOptions) JSC.Maybe(void) { // TODO: this should be concurrent. + var isatty: ?bool = null; + var is_nonblocking_tty = false; const fd = switch (switch (options.input_path) { .path => |path| bun.sys.openA(path.slice(), options.flags(), options.mode), - .fd => |fd_| bun.sys.dupWithFlags(fd_, if (bun.FDTag.get(fd_) == .none and !this.force_sync_on_windows) std.os.O.NONBLOCK else 0), + .fd => |fd_| brk: { + if (comptime Environment.isPosix and FeatureFlags.nonblocking_stdout_and_stderr_on_posix) { + if (bun.FDTag.get(fd_) != .none) { + const rc = bun.C.open_as_nonblocking_tty(@intCast(fd_.cast()), std.os.O.WRONLY); + if (rc > -1) { + isatty = true; + is_nonblocking_tty = true; + break :brk JSC.Maybe(bun.FileDescriptor){ .result = bun.toFD(rc) }; + } + } + } + + break :brk bun.sys.dupWithFlags(fd_, if (bun.FDTag.get(fd_) == .none and !this.force_sync_on_windows) std.os.O.NONBLOCK else 0); + }, }) { .err => |err| return .{ .err = err }, .result => |fd| fd, @@ -3054,13 +3069,22 @@ pub const FileSink = struct { return .{ .err = err }; }, .result => |stat| { - this.pollable = bun.sys.isPollable(stat.mode) or std.os.isatty(fd.int()); + this.pollable = bun.sys.isPollable(stat.mode); + if (!this.pollable and isatty == null) { + isatty = std.os.isatty(fd.int()); + } + + if (isatty) |is| { + if (is) + this.pollable = true; + } + this.fd = fd; this.is_socket = std.os.S.ISSOCK(stat.mode); - this.nonblocking = this.pollable and switch (options.input_path) { + this.nonblocking = is_nonblocking_tty or (this.pollable and switch (options.input_path) { .path => true, .fd => |fd_| bun.FDTag.get(fd_) == .none, - }; + }); }, } } else if (comptime Environment.isWindows) { @@ -3402,11 +3426,20 @@ pub const FileReader = struct { pub fn openFileBlob(file: *Blob.FileStore) JSC.Maybe(OpenedFileBlob) { var this = OpenedFileBlob{ .fd = bun.invalid_fd }; var file_buf: [std.fs.MAX_PATH_BYTES]u8 = undefined; + var is_nonblocking_tty = false; const fd = if (file.pathlike == .fd) - if (file.pathlike.fd.isStdio()) - file.pathlike.fd - else switch (Syscall.dupWithFlags(file.pathlike.fd, brk: { + if (file.pathlike.fd.isStdio()) brk: { + if (comptime Environment.isPosix) { + const rc = bun.C.open_as_nonblocking_tty(@intCast(file.pathlike.fd.int()), std.os.O.RDONLY); + if (rc > -1) { + is_nonblocking_tty = true; + file.is_atty = true; + break :brk bun.toFD(rc); + } + } + break :brk file.pathlike.fd; + } else switch (Syscall.dupWithFlags(file.pathlike.fd, brk: { if (comptime Environment.isPosix) { if (bun.FDTag.get(file.pathlike.fd) == .none and !(file.is_atty orelse false)) { break :brk std.os.O.NONBLOCK; @@ -3459,14 +3492,20 @@ pub const FileReader = struct { return .{ .err = Syscall.Error.fromCode(.ISDIR, .fstat) }; } - this.pollable = bun.sys.isPollable(stat.mode) or (file.is_atty orelse false); + this.pollable = bun.sys.isPollable(stat.mode) or is_nonblocking_tty or (file.is_atty orelse false); this.file_type = if (bun.S.ISFIFO(stat.mode)) .pipe else if (bun.S.ISSOCK(stat.mode)) .socket else .file; - this.nonblocking = this.pollable and !(file.is_atty orelse false); + + // pretend it's a non-blocking pipe if it's a TTY + if (is_nonblocking_tty and this.file_type != .socket) { + this.file_type = .nonblocking_pipe; + } + + this.nonblocking = is_nonblocking_tty or (this.pollable and !(file.is_atty orelse false)); if (this.nonblocking and this.file_type == .pipe) { this.file_type = .nonblocking_pipe; diff --git a/src/c.zig b/src/c.zig index 150dae475c584..5e74b804f3486 100644 --- a/src/c.zig +++ b/src/c.zig @@ -467,3 +467,4 @@ pub extern "C" fn Bun__ttySetMode(fd: c_int, mode: c_int) c_int; pub extern "C" fn bun_initialize_process() void; pub extern "C" fn bun_restore_stdio() void; +pub extern "C" fn open_as_nonblocking_tty(i32, i32) i32; diff --git a/src/feature_flags.zig b/src/feature_flags.zig index 27a173f8b2d9b..eb9d55f3e5fd7 100644 --- a/src/feature_flags.zig +++ b/src/feature_flags.zig @@ -165,3 +165,8 @@ pub const disable_auto_js_to_ts_in_node_modules = true; pub const runtime_transpiler_cache = true; pub const breaking_changes_1_1_0 = false; + +// This causes strange bugs where writing via console.log (sync) has a different +// order than via Bun.file.writer() so we turn it off until there's a unified, +// buffered writer abstraction shared throughout Bun +pub const nonblocking_stdout_and_stderr_on_posix = false;