Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(posix): ensure nonblocking file descriptor is nonblocking (#10080) #10303

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bun.js/webcore/streams.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3562,7 +3562,7 @@ pub const FileReader = struct {
this.file_type = .nonblocking_pipe;
}

this.nonblocking = is_nonblocking_tty or (this.pollable and !(file.is_atty orelse false));
this.nonblocking = is_nonblocking_tty or bun.isNonBlocking(fd.int());

if (this.nonblocking and this.file_type == .pipe) {
this.file_type = .nonblocking_pipe;
Expand Down
7 changes: 7 additions & 0 deletions src/bun.zig
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,13 @@ pub fn ensureNonBlocking(fd: anytype) void {
_ = std.os.fcntl(fd, std.os.F.SETFL, current | std.os.O.NONBLOCK) catch 0;
}

pub fn isNonBlocking(fd: anytype) bool {
if (Environment.isWindows) {
@compileError("isNonBlocking() is not supported on Windows");
}
return (std.os.fcntl(fd, std.os.F.GETFL, 0) catch 0 & std.os.O.NONBLOCK) == std.os.O.NONBLOCK;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (std.os.fcntl(fd, std.os.F.GETFL, 0) catch 0 & std.os.O.NONBLOCK) == std.os.O.NONBLOCK;
if (Environment.isWindows) {
@compileError("isNonBlocking is not supported on windows");
}
return (std.os.fcntl(fd, std.os.F.GETFL, 0) catch 0 & std.os.O.NONBLOCK) == std.os.O.NONBLOCK;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking at this, do you need any action from me? Would you like me to rebase against master and apply the above suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that would be great 👍

Copy link
Author

@alexkornitzer alexkornitzer May 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I tried to align the formatting to that of other @compileError functions within the file. I also squashed the formatting commit into the original commit to keep the git history clean.

}

const global_scope_log = sys.syslog;
pub fn isReadable(fd: FileDescriptor) PollFlag {
if (comptime Environment.isWindows) {
Expand Down
33 changes: 33 additions & 0 deletions test/regression/issue/10080.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { test, expect } from "bun:test";
import { bunEnv, bunExe, isPosix } from "harness";
import { tmpdir } from "os";
import { join } from "path";

test.if(isPosix)(
"10080 - ensure blocking stdio is treated as such in FileReader",
async () => {
const expected = "foobar\n";
const filename = join(tmpdir(), "bun.test.stream." + Date.now() + ".js");
const contents = "for await (const line of console) {console.log(`foo${line}`)}";
await Bun.write(filename, contents);
const shellCommand = `exec &> >(${bunExe()} ${filename}); echo "bar"; while read -r line; do echo $line; done`;

const proc = Bun.spawn(["bash", "-c", shellCommand], {
stdin: "inherit",
stdout: "pipe",
stderr: "inherit",
env: bunEnv,
});
const { value } = await proc.stdout.getReader().read();
const output = new TextDecoder().decode(value);
if (output !== expected) {
expect(output).toEqual(expected);
throw new Error("Output didn't match!\n");
}

proc.kill(9);
await proc.exited;
expect(proc.killed).toBeTrue();
},
1000,
);