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 running Bun.spawn on Vercel and GCP #6724

Merged
merged 9 commits into from
Oct 27, 2023
Merged

Conversation

Jarred-Sumner
Copy link
Collaborator

What does this PR do?

pidfd_open is not implemented by gVisor and by older Linux kernels. This implements a workaround where we spawn a thread, enqueue concurrent tasks and call waitpid on each Subprocess's pid. This workaround is very loosely based on libuv's default behavior.

Fixes #5803

Need to manually test that it does indeed fix the issue, but it should

How did you verify your code works?

I tested manually + added a hidden environment variable to let us manually enable this for testing purposes

@@ -476,11 +476,23 @@ pub fn scoped(comptime tag: @Type(.EnumLiteral), comptime disabled: bool) _log_f
defer lock.unlock();

if (Output.enable_ansi_colors_stderr) {
out.print(comptime prettyFmt("<r><d>[" ++ @tagName(tag) ++ "]<r> " ++ fmt, true), args) catch unreachable;
buffered_writer.flush() catch unreachable;
out.print(comptime prettyFmt("<r><d>[" ++ @tagName(tag) ++ "]<r> " ++ fmt, true), args) catch {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was to silence debug logs from throwing "PipeError"

}

pub fn shouldUseWaiterThread() bool {
return @atomicLoad(bool, &should_use_waiter_thread, .Monotonic);
Copy link
Member

Choose a reason for hiding this comment

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

should be fine but why not use std.atomic and/or atomic store when setting

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

@Jarred-Sumner 2 files with test failures on linux-x64-baseline:

  • test/js/bun/spawn/spawn.test.ts
  • test/js/bun/util/filesink.test.ts

View test output

#b016c5ce86bf3ee6996a1b7d86e6637afe6df92e

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

@Jarred-Sumner 2 files with test failures on linux-x64:

  • test/js/bun/spawn/spawn.test.ts
  • test/js/bun/util/filesink.test.ts

View test output

#b016c5ce86bf3ee6996a1b7d86e6637afe6df92e

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

@Jarred-Sumner 5 files with test failures on bun-darwin-aarch64:

  • test/cli/install/bunx.test.ts
  • test/integration/next/default-pages-dir/test/dev-server.test.ts
  • test/js/bun/test/test-test.test.ts
  • test/js/bun/util/filesink.test.ts
  • test/js/node/watch/fs.watch.test.ts

View test output

#b016c5ce86bf3ee6996a1b7d86e6637afe6df92e

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

@Jarred-Sumner 6 files with test failures on bun-darwin-x64-baseline:

  • test/integration/next/default-pages-dir/test/dev-server.test.ts
  • test/js/bun/spawn/spawn-streaming-stdout.test.ts
  • test/js/node/fs/fs.test.ts
  • test/js/node/watch/fs.watchFile.test.ts
  • test/js/third_party/webpack/webpack.test.ts
  • test/js/web/timers/setTimeout.test.js

View test output

#b016c5ce86bf3ee6996a1b7d86e6637afe6df92e

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

@Jarred-Sumner 4 files with test failures on bun-darwin-x64:

  • test/js/node/fs/fs.test.ts
  • test/js/node/watch/fs.watchFile.test.ts
  • test/js/third_party/webpack/webpack.test.ts
  • test/js/web/timers/setTimeout.test.js

View test output

#b016c5ce86bf3ee6996a1b7d86e6637afe6df92e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pidfd_open(2) system call is not supported
2 participants