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

Process is stuck after attempting to terminate already-exited worker #11335

Closed
zawodskoj opened this issue May 24, 2024 · 3 comments · May be fixed by #11376
Closed

Process is stuck after attempting to terminate already-exited worker #11335

zawodskoj opened this issue May 24, 2024 · 3 comments · May be fixed by #11376
Labels
bug Something isn't working

Comments

@zawodskoj
Copy link
Contributor

zawodskoj commented May 24, 2024

What version of Bun is running?

1.1.9-debug+bb13798d9

What platform is your computer?

Microsoft Windows NT 10.0.22631.0 x64

What steps can reproduce the bug?

While trying a few things in debug-build of bun I stumbled upon this bug, which is likely related to #9998 (the reason I got into debugging in the first place)

setTimeout can be arbitrarily large number - it only need to wait for worker to exit before we can call terminate()

import { Worker, isMainThread } from "node:worker_threads"

if (isMainThread) {
  const worker = new Worker(__filename);

  setTimeout(() => worker.terminate(), 60);
} else {
  // worker does not need to do anything
}

What is the expected behavior?

Process does not hang and exits correctly

What do you see instead?

Process hangs

Additional information

I could not build bun in WSL (while using ManjaroWSL bun-debug crashes every single time with SIGABRT), so I was able to test Windows version only, but it is likely to reproduce on Linux too

Debug logs: https://gist.github.com/zawodskoj/d6c2569db71ac23e131502266d8e6769

@zawodskoj zawodskoj added the bug Something isn't working label May 24, 2024
@zawodskoj
Copy link
Contributor Author

zawodskoj commented May 25, 2024

Update: this applies to files as well - stdin and piping is not required

import { Readable, pipeline, PassThrough } from 'node:stream'
import { Worker, isMainThread } from "node:worker_threads"
import { createReadStream } from "node:fs"

if (isMainThread) {
  const worker = new Worker(__filename);

  setTimeout(() => worker.terminate(), 60);
} else {

  pipeline(
      Readable.toWeb(createReadStream(__filename)),
      new PassThrough(),
      process.stdout,
      () => { },
  )
}

@zawodskoj
Copy link
Contributor Author

zawodskoj commented May 26, 2024

The most bizarre thing I noticed while debugging is that curly braces around setTimeout lambda body actually prevent bug to happen

Replacing
setTimeout(() => worker.terminate(), 60);
with
setTimeout(() => { worker.terminate() }, 60);
resulting in process not being stuck

And there's no need to do any IO, repro works just fine when worker does not do anything - it seems only required condition is to request termination on worker that is already exited

@zawodskoj zawodskoj changed the title Process is stuck after terminating worker thread while performing IO Process is stuck after attempting to terminate already-exited worker May 26, 2024
@zawodskoj
Copy link
Contributor Author

Closed in favor of #11377 (which is the real problem)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant