Skip to content

worker_threads: don't abort when terminate() interrupts a lazy property builder - #33418

Closed
robobun wants to merge 3 commits into
mainfrom
farm/912391f8/lazy-property-termination
Closed

worker_threads: don't abort when terminate() interrupts a lazy property builder#33418
robobun wants to merge 3 commits into
mainfrom
farm/912391f8/lazy-property-termination

worker: defer termination while a lazy property builder runs

148063a
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 6, 2026 in 15m 50s

Code review found 1 important issue

Found 3 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/jsc/bindings/BunObject.cpp:360-366 Bun.sql/Bun.SQL/Bun.postgres lazy builders also enter JS and need DeferTerminationForAWhile

Annotations

Check failure on line 366 in src/jsc/bindings/BunObject.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

Bun.sql/Bun.SQL/Bun.postgres lazy builders also enter JS and need DeferTerminationForAWhile

`Bun.sql`, `Bun.postgres` (`defaultBunSQLObject`) and `Bun.SQL` (`constructBunSQLObject`) also enter JS via `requireId()` → `generateModule()` → `JSC::profiledCall` on first touch, and both use `DECLARE_THROW_SCOPE` + `RETURN_IF_EXCEPTION(scope, {})` — the same shape as `constructBunShell` pre-fix. The PR description says the remaining `PropertyCallback` builders "only allocate", but these three don't; they need the same `DeferTerminationForAWhile` guard (and ideally a slot in the `lazyPropertie