fix: explicitly start MessagePort

[nazarhussain]

Bun js is getting popularity and a lot of tools are started to support it. And tinypool is dependency of vitest which is widely used across the ecosystem. So it's really important that vitest supports Bun runtime so other projects can also test their specs more easily.

The only limitation I observed Vitest supporting Bun is the tinypool support for it. I earlier created another test pool implementation for vitest to support Bun https://github.com/nazarhussain/vitest-in-process-pool but I believe the best direction would be to make the tinypool compatible with Bun, so rest of the ecosystem align automatically.

Not everything is working but a lot. Here is a glimps of it.

 Test Files  7 failed | 9 passed (16)
      Tests  8 failed | 77 passed | 1 skipped (86)
     Errors  1 error
   Start at  15:27:19
   Duration  22.60s (transform 115ms, setup 0ms, collect 196ms, tests 22.20s, environment 0ms, prepare 29ms)

There are few limitations though:

1. Multi process pool (it works fine as far as tinypool implementations concerns) but there is an issue in Vitest code that triggers TDZ issue and we can't test it.
2. Use of atomics is not yet compatible with the Bun, so setting useAtomics to false will work fine.
3. Use of resourceLimits not yet supported by Bun.

[AriPerkkio]

Linking here the previous attempts/frustration:

• #70
• #73

But I'm still not sure if this is the right approach. We don't have if (isDeno) checks either - tinypool just works there. This is really something Bun should fix instead of us. Tinypool is compatible with NodeJS-like runtimes.

But I'm happy to see what kind of changes are required here and if we can isolate them out of the common code.

[nazarhussain]

This is really something Bun should fix instead of us. Tinypool is compatible with NodeJS-like runtime.

This is based on assumption that every other JS runtime like Bun should have 100% compatibility with NodeJS. In reality it is and it will never be true, there will be compatibility differences across different runtimes. And that's where libraries like tinypool can make small efforts to make those work on those runtimes.

But I'm happy to see what kind of changes are required here and if we can isolate them out of the common code.

There are three types of changes:

1. Major compatibility difference
   I tried to isolate such into seprate code files, e.g. bun-process-worker.ts have completely different implementation for ProcessWorker for Bun.

2. Syntax compatibility
   On some places there is syntax/dsl difference, e.g. Node supports port.onmessage and port.on('message' syntax, but the Bun works well with first one. So used that common syntax.

3. Branching
   Some places we have to switch logic based on runtime, that's only where we have mix code and use of isBun.

[AriPerkkio]

This is based on assumption that every other JS runtime like Bun should have 100% compatibility with NodeJS.

Yes, that is somewhat the assumption on runtimes that have pages like Bun's Node.js compatibility. I wouldn't expect 100% though as there will always be weird edge cases.

Previously I've noticed that Bun's node:child_process and node:worker_threads support is not Node compatible, and Bun's Worker is unstable and causes unexpected crashes. I've not tested the Subprocess of Bun that this PR focuses on. 🤔

If Subprocess doesn't leak memory and crash like Worker, we could do something similar as attempted in #73. Have specific src/runtime and src/entry for it, instead of modifying the Node compatible process and worker that Node and Deno use.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/tinypool@116

commit: b413db0

[AriPerkkio]

This is how Tinypool support in Bun should be achieved: oven-sh/bun#19940, exactly as mentioned in #116 (review).

Let's see how the progress goes there first.

[nazarhussain]

This is how Tinypool support in Bun should be achieved: oven-sh/bun#19940, exactly as mentioned in #116 (review).

Let's see how the progress goes there first.

I encourage any contribution to Bun runtime, but I personally feel that approaching the main problem with support bun for library xyz is not the right approach. Rather we should contribute for specific problem or feature.

And also expecting Bun or any other runtime to follow NodeJS even when NodeJS is not aligned with JS specs (see (comment)[https://github.com/oven-sh/bun/issues/19863#issuecomment-2929201194]) will help grow that runtime. And no matter what the cross runtime compatibility issues will remain in fine details, so libraries should also be covering that compatibility part.

[AriPerkkio]

We are more than happy to add changes that are required by JS specs, that NodeJS does not follow, such as calling port.start() manually.

If we indeed want to add runtime specific conditions in libraries, I would also like to see these changes in Piscina that is the upstream of Tinypool. It would be easier to adapt those changes then.

Edit: Actually it looks like Piscina maintainers are waiting for support from Bun's side instead: piscinajs/piscina#787 (comment).

[nazarhussain]

@AriPerkkio This PR now contains the minimum possible change to support Bun. Please review.

❯ bun run --bun test run 
$ vitest run

 RUN  v3.1.3 /Users/nazar/Hub/Lodestar/Projects/tinypool

 ✓ test/simple.test.ts (25 tests) 5890ms
   ✓ workerId should never be more than maxThreads=1  413ms
   ✓ workerId should never be more than maxThreads  397ms
   ✓ worker count should never be below minThreads when using isolateWorkers  386ms
   ✓ workerId should never be duplicated  4266ms
 ✓ test/task-queue.test.ts (8 tests) 2064ms
   ✓ queued tasks can be cancelled  2017ms
 ✓ test/runtime.test.ts (11 tests) 251ms
 ✓ test/move.test.ts (7 tests) 37ms
 ✓ test/resource-limits.test.ts (5 tests | 1 skipped) 267ms
   ↓ resourceLimits causes task to reject [process resourceLimits are not yet supported in Bun]
 ✓ test/atomic.test.ts (2 tests | 2 skipped) 0ms
   ↓ coverage test for Atomics optimization [Atomics are not supported in Bun]
   ↓ avoids unbounded recursion [Atomics are not yet supported in Bun]
 ✓ test/isolation.test.ts (4 tests) 4559ms
   ✓ worker_threads > running workers can recycle after task execution finishes  2023ms
   ✓ child_process > running workers can recycle after task execution finishes  2040ms
 ✓ test/cpu-count.test.ts (5 tests) 6ms
 ✓ test/pool-destroy.test.ts (3 tests | 1 skipped) 7ms
   ↓ cleans up async resources [AsyncHooks are not yet supported in Bun]
 ✓ test/termination.test.ts (3 tests | 1 skipped) 249ms

 ✓ test/uncaught-exception-from-handler.test.ts (4 tests) 577ms
   ✓ uncaught exception in immediate after task yields error event  511ms
 ✓ test/async-context.test.ts (1 test | 1 skipped) 0ms
   ↓ postTask() calls the correct async hooks [AsyncHooks are not yet supported in Bun]
 ↓ test/worker-stdio.test.ts (2 tests | 2 skipped)
 ✓ test/options.test.ts (3 tests) 71ms
 ✓ test/idle-timeout.test.ts (1 test) 2036ms
   ✓ idle timeout will let go of threads early  2036ms
 ✓ test/globals.test.ts (2 tests) 43ms

 Test Files  15 passed | 1 skipped (16)
      Tests  78 passed | 8 skipped (86)
   Start at  14:51:11
   Duration  16.51s (transform 127ms, setup 0ms, collect 181ms, tests 16.06s, environment 0ms, prepare 25ms)

[nazarhussain]

There is a weird error in Bun runtime which I yet can't figured out. Do you have any idea what it would be?

TypeError: undefined is not a constructor (evaluating 'new Tinypool({
    filename: new URL(import.meta.url, import.meta.url).href,
    runtime: "child_process",
    isolateWorkers: !0,
    minThreads: cpus().length - 1,
    maxThreads: cpus().length - 1
  })')

[nazarhussain]

Rebased the branch with upstream main. Now all tests passes for Bun.

 Test Files  16 passed | 1 skipped (17)
      Tests  85 passed | 8 skipped (93)
   Start at  21:43:22
   Duration  17.05s (transform 117ms, setup 0ms, collect 204ms, tests 16.66s, environment 0ms, prepare 23ms)

There are only one item left to figure out further, but seems not integral behavior issue.

https://github.com/tinylibs/tinypool/pull/116/files#diff-c2b19b8aa6aa53863ead43d9521dd770672764a347b39054ad14c9a19045de33R64

I am looking on it, if you have nay thoughts please share. Please review rest of the code.

[AriPerkkio]

Changes under src/** look good - there's not too many exceptions just for Bun. I would rather remove the changes of test/** and CI stuff. Did you try if node:worker_threads works in Bun? That's the runtime: 'worker_threads' in Tinypool.

Like I mentioned in #116 (comment), for Bun users it would be best to use either Subprocess or Worker from Bun directly instead of trying to make Tinypool's NodeJS runtimes compatible. I would expect Bun's "native" APIs be more efficient than Node compatible APIs.

[nazarhussain]

@AriPerkkio Sorry last week was very busy in BBW (Berlin Blockchain Week), so could not get into this feedback. But I would dedicate time this week to get this PR going.

Related to your comment about using Bun Subprocess and Worker, I did some benchmarking and observed that the performance overhead for the compatibility layer is not that much over native implementation. So for keeping this for now would be sufficient. In our projects we are extensively working on Bun, so if we observe any major change we can open and other PR for it. What do you think?

Related to to you other point I would rather remove the changes of test/**, I don't really get it. Do you want to split the test files for nodejs and bun? As those changes make sure non relevant tests for Bun does not execute and fail eventually.

[AriPerkkio]

Vitest run with preview, looks good: https://github.com/AriPerkkio/vitest/actions/runs/15687002111 ✅

[AriPerkkio]

Looks good, thanks!

I had to remove the Bun CI as it was failing tests constantly, even when using --fileParallelism=false.