fix(ext/node): implement proper resourceLimits for node:worker_threads#32430
Conversation
Implements the `resourceLimits` option for `node:worker_threads` Worker constructor, resolving denoland#26156. Previously, resourceLimits was accepted but silently ignored. - Thread `resourceLimits` from JS through `op_create_worker` to the V8 isolate's `CreateParams::heap_limits()` - Install a near-heap-limit callback that gracefully terminates the worker instead of crashing the entire process with a fatal V8 OOM - Emit `ERR_WORKER_OUT_OF_MEMORY` error with exit code 1, matching Node.js behavior - Expose correct `resourceLimits` inside the worker via serialized metadata, and clear to `{}` after worker exits - Enable `test-worker-resource-limits.js` node_compat test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix JSON key order in main.out to match actual object property order - Assert ERR_WORKER_OUT_OF_MEMORY error code and message in OOM test - Fix expected OOM exit code to 1 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
…urceLimits - Replace incorrect `heap_limits()` call with individual setters: `set_max_old_generation_size_in_bytes`, `set_max_young_generation_size_in_bytes`, `set_code_range_size_in_bytes` (requires v8 crate with ResourceConstraints API) - Handle `stackSizeMb` by setting OS thread stack size via `std::thread::Builder::stack_size()` - Add `ResolvedResourceLimits` struct and `op_worker_get_resource_limits` op to pass V8 defaults back to the worker JS context - Read back resolved values from CreateParams getters for unspecified fields - Update worker_threads.ts to use resolved limits from Rust instead of metadata - Update spec test expected outputs - Disable node_compat test-worker-resource-limits.js (needs v8.getHeapSpaceStatistics) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Upgrade v8 crate from path dep to published 146.2.0. Fix timing issue where resolved resource limits were stored in op_state after bootstrap, but the worker_threads polyfill reads them during bootstrap. Now uses WebWorker::from_options + manual bootstrap to inject limits before init. Remove plan.md (disallowed as top-level file by linter). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
resourceLimits for node:worker_threadsresourceLimits for node:worker_threads
… for CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
resourceLimits for node:worker_threadsresourceLimits for node:worker_threads
|
solid PR overall. few things:
looks good otherwise, clean impl that matches node behavior |
This is fine - we're calling
It will be addressed in #32483. |
|
Checked against Node source. A few behavioral differences:
Otherwise looks good, clean impl. |
| if has_resource_limits { | ||
| let ts_handle = worker.js_runtime.v8_isolate().thread_safe_handle(); | ||
| let oom_flag = worker.oom_triggered.clone(); | ||
| worker.js_runtime.add_near_heap_limit_callback( |
There was a problem hiding this comment.
fwiw this callback runs when v8 performs a last-resort gc. that gc might reclaim enough space that js is able to continue running normally.
…ts properly Address PR review feedback: add comment explaining sub-MB truncation is intentional for resolved resource limits, and use WorkerOptions["resourceLimits"] type instead of `any`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bf92b8d to
fe804d1
Compare
Summary
Implements proper
resourceLimitssupport fornode:worker_threads, matching Node.js behavior:set_max_old_generation_size_in_bytes,set_max_young_generation_size_in_bytes,set_code_range_size_in_bytesstd::thread::Builder::stack_size()forstackSizeMbop_worker_get_resource_limitsop, soresourceLimitsinside the worker always shows actual values (matching Node.js behavior)ERR_WORKER_OUT_OF_MEMORYerror codeworker.bootstrap()so the polyfill can read them during initCloses #26156
Closes #10036