W-23692110: Multiple isolated DataWeave engines per process (Node + Python) - #157
W-23692110: Multiple isolated DataWeave engines per process (Node + Python)#157mlischetti wants to merge 151 commits into
Conversation
|
Pushed remediation for the two code reviews (
A final whole-branch review across all 14 commits came back clean (no Critical/Important findings); the two Minor findings it raised (native-level test for the "Unknown engine handle" JSON contract, and stray review-notes files) are fixed in the last two commits. |
Root-cause fix for the three findings in the sixth PR #157 follow-up review: model the DataWeave instance lifecycle explicitly (uninitialized/ready/ cleaning-up) instead of a single boolean, make C-side stream/transform admission atomic under g_mutex, and validate napi_get_value_int64 at the handle-read sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Root-cause fix for the three findings in the sixth PR #157 follow-up review: model the DataWeave instance lifecycle explicitly (uninitialized/ready/ cleaning-up) instead of a single boolean, make C-side stream/transform admission atomic under g_mutex, and validate napi_get_value_int64 at the handle-read sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2a038f2 to
d504c0f
Compare
Round 11 hardening pushed (
|
| # | Finding | Commit |
|---|---|---|
| — | Extract bridge_begin_op_locked (atomic admission-time pin) |
7f24fc7 |
| #2 | Pin engine at admission for streaming + transform (in_flight++ in the same locked section as g_active_ops++; released exactly once on every early-return) |
27cc714 |
| #3 | Pin engine for the synchronous runScriptEngine |
4950380 |
| #1 | Register an env cleanup hook for every engine (not just resolver-backed); extend the owner-thread destroyEngine guard to fire for any record |
fd49ad1 |
| #5 | Register process beforeExit/exit hooks once per module (never-reset guard) instead of per-singleton |
58c2690 |
| #6 | Real-addon integration tests for the *_engine unknown/destroyed-handle envelope + best-effort run-vs-destroy guard |
cacb41b, 6865803 |
Full Node suite: 885 passed / 59 skipped / 0 failed. Passed a whole-branch review (correctness of the pin across all three run paths, exactly-once release on every early-return, lifecycle-invariant preservation).
Two notes for reviewers
-
Contract change (finding Create LICENSE #1): the owner-thread
destroyEngineguard now keys on "a record exists," not on resolvernapi_refstate — because every engine now carries an env cleanup hook (env-affine state), so a resolver-less engine is also only destroyable from its creating thread. This supersedes the old "resolver-less engines are destroyable from any thread" invariant described in some earlier design docs (docs/superpowers/specs/2026-08-19-worker-teardown-dangling-resolver-ctx-design.mdand the two2026-08-18-*docs).bridge_finalize'snapi_refdeletion stays resolver-gated. -
dwlib C ABI break (finding Set in/out mimetypes as input parameters, env vars #4) is intentional and documented — no compat shims. The branch replaces the
run_script_*_with_resolverexports with the*_enginemulti-engine entrypoints (legacy singletonsrun_script/run_script_callback/run_script_input_output_callbackpreserved) and adds actxparameter toResolveModuleCallback. dwlib is consumed by this repo's own Python/Node bindings in lockstep.
Follow-up not in this round: the runTransform TOCTOU (general review #1) is made memory-safe by the C-side admission pin (worst case is a resolved Unknown engine handle envelope, not a UAF). The additional JS-side recheck/lease the reviewer suggested was deliberately not added — the authoritative pin lives in C. Can add the JS belt-and-suspenders as a separate change if desired.
Round 12 — worker ref-leak & teardown-race hardeningPushed
Concurrency invariants (all re-derived and confirmed in a whole-branch review): exactly-one Tests: 895 passed / 59 skipped / 0 failed. |
Round 13 — per-env init-reference ownership (fixes review #5)Pushed What changed. Replaced the process-global init-reference model — which assumed a strict 1 Mechanism.
Review. Each task passed a spec + quality gate; the whole-branch final review (on the most capable model) independently re-derived the Tests. Full Node suite 897 passed / 59 skipped / 0 failed (added 2 integration tests). The Java layer and Known coverage gap (documented in the test file). The two new |
Round 14 — review #5 remediation (all 7 findings)Addresses every finding from follow-up code review #5 (reviewed head
Invariant preserved throughout: Java side and the legacy singleton entrypoints are untouched this round. 🤖 Generated with Claude Code |
Round 15 — external review #6 remediationAddressed all 8 code findings from the latest follow-up review (
On #5's residual (accepted, documented in-code): if a teardown fails and no later #9 (Python-binding scope) — intentionally not split. The review noted the PR bundles Python-binding modernization alongside the Node multi-engine work. That bundling is intentional for this PR and will not be split out in this round; the Python work is being tracked as part of the same effort. Happy to revisit if a reviewer feels strongly, but flagging it here so the decision is explicit. A whole-branch review of the combined round-15 changes traced the full |
Root-cause fix for the three findings in the sixth PR #157 follow-up review: model the DataWeave instance lifecycle explicitly (uninitialized/ready/ cleaning-up) instead of a single boolean, make C-side stream/transform admission atomic under g_mutex, and validate napi_get_value_int64 at the handle-read sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aaeafb9 to
6fb5603
Compare
Review #7 remediation pushed (
|
| # | Sev | Fix | Commit |
|---|---|---|---|
| #1 | High | Detach the phantom GraalVM thread on a failed (!=0) graal_tear_down_isolate, in both cleanup_thread_fn and teardown_waiter_thread_fn (detach only on the failure branch — never on success/UAF or attach-failure) |
55b2418 |
| #2 | Med | Arm the stranded-teardown retry after a failed init-hook rollback so the next initialize() reclaims the isolate instead of wedging |
ff3b6bb |
| #3 | Med | Model initialize()'s ffi.cleanup() rollback as pending state — observe it (no unhandledRejection) and gate a concurrent initialize() with "cleanup is in progress"; still throws the original error synchronously |
ae35089 |
| #6 | Low | Track native stream rejection by settlement state (startRejected) instead of the startError !== undefined value sentinel, so Promise.reject(undefined) propagates |
eed6ee4 |
| #7 | Low | Fail the worker-lifecycle test when balancing cleanup fails on a passing body (re-throw only when bodySucceeded) |
c258463 |
| #5 | Med | State the final-reference condition in the instance cleanup() docs (resolves after full isolate teardown only when releasing the last instance) |
6462d96 |
| #4 | Med | Root README: await cleanup in the lifecycle example; document the real beforeExit+exit hook pair and signal caveat |
6911de9 |
The final review also tightened the #2 inline comment (c54ff65): that acquire-failure path leaves g_initialized==0, so the retry recovers a transient teardown failure but a persistent graal_tear_down_isolate failure strands the isolate to process exit (pre-existing best-effort degradation) — the prior "adopted by the fast path" wording overstated it.
On finding #8 (Med) — Python-binding modernization scope
Acknowledged, and we agree the Python pytest setup / module extraction / TCK / CI / docs are logically a separate concern from the Node engine-lifecycle work. We're deferring the split to its own follow-up PR rather than doing git surgery on this branch now: the Node lifecycle changes have been through seven review rounds against this exact history, and re-splitting risks disturbing that reviewed state for a packaging reorg. The follow-up PR will carry the Python modernization on its own so its CI/rollout/rollback can be reasoned about independently. This branch's remaining diff remains the Node engine/lifecycle work plus these remediations.
Review #8 remediation — all 7 findings addressed (pushed
|
| # | Finding | Fix | Commit |
|---|---|---|---|
| 1 | High — napi_initialize deadlocks after a repeated stranded-teardown failure (blocks on a uv_cond_wait nobody signals) |
After the retry, detect the unrecoverable stranded state (g_isolate != NULL && !g_initialized && TEARDOWN_NONE) and throw a deterministic error instead of entering the wait loop. g_teardown_needed stays armed and g_ref_count is untouched. |
9fc728f |
| 2 | Med — a synchronous rollback ffi.cleanup() throw strands the instance in "cleaning-up" |
Route the release through a promise boundary so a synchronous throw is normalized to a rejection that still settles the .finally state reset; the caller still synchronously sees the original engine-creation error. |
85d06ae |
| 3 | Med — worker-lifecycle balancing cleanup skips ffi.cleanup() when destroyEngine throws |
Capture the destroy error but always run ffi.cleanup(); surface a balancing failure only when the test body succeeded. |
eccc9d9 (+ 1831ccd) |
| 4 | Med — README teaches invalid terminal-metadata retrieval (generator.return() after for await) |
Document manual next() iteration capturing the terminal { done: true, value: StreamingResult }. |
faf0e78 |
| 5 | Med — README overstates runTransform async input as constant-memory |
Switch large-file examples to a synchronous generator, add the sync-vs-async pre-buffering note, and qualify the Performance claims. | c7799e2 (+ 8378029) |
| 6 | Med — external-modules.md examples omit required cleanup |
Wrap the 5 complete examples in try/finally { await dw.cleanup(); }; label the 2 genuine fragments. |
9eec6b2 |
| 7 | Med — root README overstates cleanup as an unconditional process-wide drain | Qualify the §4 and §9 comments: the drain/teardown happens only when the final shared reference is released. | 8347516 |
Notes
- On finding Create LICENSE #1 and my review Update .travis.yml #7 sign-off: my review Update .travis.yml #7 final review wrongly cleared this as a benign "lingers-to-process-exit" degradation — I conflated the
napi_initializewait path with theisolate_ref_release_n_lockedrelease-path twin (which is benign and adoptable). Finding Create LICENSE #1 was a real hard deadlock (uv_cond_waitholdingg_mutexwith no remaining signaller), and it is now closed. The final review traced the new guard against every state reachable at the wait-loop entry to confirm it fires only in the stranded case and never on a cold start, a healthy re-init, or a legitimate in-flight teardown. - Finding Unable to resolve reference to payload error #2 fix detail: the review's suggested
Promise.resolve().then(() => ffi.cleanup())defersffi.cleanup()into a microtask, which breaks two existing tests that assert synchronous invocation. The shipped fix uses an equivalenttry/catch→Promise.rejectnormalization that preserves the synchronous invocation timing while achieving the same intent. - Two Minor accuracy nits raised by the final review (test double-fault error-masking; a
readFileSyncbounded-memory caveat) are folded into1831ccd/8378029. - Scope unchanged: Node-binding-only; no Python, Java, or legacy-singleton entrypoints touched; handle width stays
long long.
Addresses GUS W-23692110, discovered while implementing Node.js external module support (#154). native-lib's ScriptRuntime is a static singleton with a write-once resolver, so a second DataWeave instance in one Node process silently reuses the first instance's resolver instead of getting its own. Design: turn ScriptRuntime into a handle-addressable registry of per-instance engines (one shared GraalVM isolate, following the pattern native-cli's NativeRuntime already uses), with a per-handle resolver bridge in the Node C addon. Python is out of scope here (tracked as a follow-up) since it already gets isolation via one isolate per instance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…egression test Rewires ffi.ts and dataweave.ts to call the new handle-based N-API methods (createEngine/createEngineWithResolver/destroyEngine/ runScriptEngine/runScriptStreamingEngine/runScriptTransformEngine) added in Task 3, removing runWithResolver. Each DataWeave instance now owns its own engineHandle, created on initialize() and destroyed on cleanup(), so multiple instances with different resolvers no longer cross-talk in the same process. Adds independent-engines.test.ts proving two resolver-backed instances resolve only their own modules, that a genuine script error on the new handle-based run() path surfaces as success:false rather than an unhandled throw (runScriptEngine now returns "" instead of throwing on a NULL native result), and that runStreaming/runTransform correctly thread the handle through addon.c's argument-shifted N-API wiring. Deletes the now-obsolete first-resolver-wins regression test and fixture, and rewrites dataweave-resolver.test.ts so each test builds its own minimal resolver map instead of sharing a process-wide "first resolver wins" module map.
…itialize() failure If ffi.initialize() succeeded but engine creation (createEngine/ createEngineWithResolver) then threw, this.initialized stayed false, so cleanup()'s early-return guard meant ffi.cleanup() was never called -- permanently leaking that instance's increment of the native library's ref-counted handle. initialize()'s catch block now releases that ref-count itself (ffi.cleanup()) when ffi.initialize() already succeeded, before wrapping and re-throwing. Adds tests/unit/dataweave-initialize.test.ts, a new unit-lane test (mocked ffi module, no dwlib required) exercising this exact sequencing bug plus the surrounding invariants: no cleanup() call when ffi.initialize() itself fails, no residual state after a failed attempt, and no spurious cleanup() call on the successful path.
…ps (F1, F2) Resolver-backed engine bridges could be freed while a background streaming/ transform uv_thread still dereferenced them via resolve_module_callback (F1), and napi_cleanup deleted thread-affine napi_refs from whatever thread made the last release (F2, undefined behavior across Workers). F1: add in_flight/destroy_pending accounting (under g_mutex). Streaming/transform setup pins the bridge via bridge_begin_op before spawning the worker thread; the completion sentinel releases it via bridge_end_op on the owner thread. destroyEngine unlinks immediately but defers the free (napi_ref delete + struct free) to the last draining op when in_flight > 0. F2: register a per-env cleanup hook (napi_add_env_cleanup_hook) per bridge at creation so each Worker/main env disposes its own napi_ref on its own thread; destroyEngine removes the hook before an early free. napi_cleanup no longer touches g_bridges and only performs the process-global GraalVM isolate teardown once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…k (F3, F4) create_engine/create_engine_with_resolver are GraalVM @CEntryPoints; if Java construction throws, the entrypoint returns the long long default value (0) instead of propagating. Treat any handle <= 0 as invalid: throw an N-API error and unwind the bridge (delete napi_ref, free struct) before it's ever linked into g_bridges or given a cleanup hook, instead of returning/inserting a bogus handle. Also fix a resolver-source buffer leak: if the malloc for the tracking node itself fails, the buffer was previously left untracked and unfreeable. resolver_results_track now reports tracking failure so resolve_module_callback can free the buffer and report "unresolved" instead of leaking it.
… path The handle <= 0 rejection path did manual napi_delete_reference + free(bridge) instead of bridge_finalize, so any resolver-callback buffers already tracked via resolver_results_track (if resolve_module_callback ran during a failed eager module setup before construction was reported as failed) were leaked. bridge_finalize already frees tracked buffers before freeing the struct and is a safe drop-in here since the bridge was never linked into g_bridges or given a cleanup hook at this point.
Node addon.c: fail initialize() with a clear message when dwlib lacks the per-engine symbols (create_engine, create_engine_with_resolver, destroy_engine, run_script_engine, run_script_callback_engine, run_script_input_output_callback_engine) instead of deferring to a confusing per-call error, since every initialize() now creates an engine. CallbackWeaveResourceResolver.resolve(): suppress exception detail by default and only log e.getMessage() when DATAWEAVE_RESOLVER_DEBUG=1, matching the C-side resolve_module_callback policy in addon.c. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…free The default (non-debug) branch of CallbackWeaveResourceResolver.resolve()'s catch block still logged the module path unconditionally, which is dynamic, resolver-controlled content. Drop path too in the default branch so the log line is fully static, matching the C-side resolve_module_callback's actual default behavior in addon.c. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds four resolver-backed integration tests to dataweave-resolver.test.ts that exercise paths untested by prior remediation commits: - a throwing resolveModule() causes run() to fail cleanly (success:false) rather than crash, exercising resolve_module_callback's exception catch/clear/log-gated-by-DATAWEAVE_RESOLVER_DEBUG path. - a resolver-backed instance's initialize -> cleanup -> initialize cycle still resolves a custom module afterwards (fresh engine_bridge_t). - cleanup() raced against an in-flight resolver-backed runStreaming() does not crash -- the regression test for the F1 in-flight-refcount fix, started deterministically by calling gen.next() without awaiting it before calling cleanup(), so the native call is already handed to the libuv worker thread when cleanup() runs on the JS thread. - run() after cleanup() throws DataWeaveError via dataweave.ts's ensureInitialized() guard (the TS-level half of the destroyed/unknown engine handle contract).
Extracts the "Unknown engine handle" JSON literal shared by run_script_engine, run_script_callback_engine, and run_script_input_output_callback_engine into a single package-visible constant (NativeLib.UNKNOWN_ENGINE_HANDLE_JSON), so the exact error contract can be asserted from a plain JVM unit test. The @centrypoint methods themselves can't be exercised directly from a JVM test since their GraalVM word-type parameters (IsolateThread, CCharPointer) only resolve inside a compiled native image. Adds ScriptRuntimeTest#unknownEngineHandleProducesExactErrorJson, which combines that constant assertion with the existing proof that ScriptRuntime.get() returns null for an unregistered handle.
These were internal review artifacts incidentally committed during remediation work (one references a local temp worktree path); they aren't product documentation and shouldn't ship in the repo.
The follow-up PR-157 review found that DataWeave.cleanup() can deadlock the process when called while a runStreaming()/runTransform() operation is still in flight: isolate teardown blocks the JS thread that a mid-delivery worker's threadsafe-function call depends on. This design makes teardown async and wait for active ops to drain via a dedicated waiter thread, instead of blocking inline.
8378029 to
136e914
Compare
…oundary; cover malformed streaming/transform raw-ffi args (review #9 #6) The transform entrypoint silently coerced any non-string inputCharset (objects, numbers, booleans) to NULL, unlike the four non-nullable string args which throw. inputCharset is nullable, so null/undefined still map to "no charset", but any other type is now a caller error that fails closed via TRANSFORM_FAIL. The JS binding only ever sends string|null (dataweave.ts: opts?.charset ?? null), so the production path is unaffected. Also completes the round-7 #2 malformed-arg sweep: adds coverage for streaming inputsJson and transform inputsJson/inputName/inputMimeType/charset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite 2026-08-07-native-lib-multi-engine-design.md to reflect the converged final state, folding all review-round hardening decisions into the relevant sections (concurrency & lifecycle model, architecture, ABI break, testing posture). Delete the 12 per-round hardening docs; a provenance appendix maps each round to the section it was folded into. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…move ScriptRuntime singleton Design spec for extending the handle-based multi-engine model to the Python binding using one unified model (shared process-wide isolate + N handle- addressed engines) and eliminating the ScriptRuntime singleton entirely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rypoints (W-23692110) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ine dispatch (W-23692110) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… (W-23692110) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e attachment (W-23692110) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…il-closed guard (W-23692110) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…solver bound at init (W-23692110) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…92110) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…23692110) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oss-thread teardown cannot hang; unregister resolver token on failed init (W-23692110) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d impl (final review #3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mlischetti
changed the title
Fold the Python-unification design (2026-08-26) into the canonical 2026-08-07 native-lib multi-engine design, retitled to cover both bindings. Removes the superseded/no-longer-true Python hedges and states the unified shipped reality directly: shared refcounted isolate + N handle-addressed engines driven by both bindings through the identical *_engine C ABI. Adds first-class Python sections (unification rationale, reuse boundary, attach-on-demand lifecycle & teardown reflecting the final-review bootstrap-detach fix, error handling, testing) and the shared six-invariant engine lifecycle contract. Deletes the now-merged 2026-08-26 doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ins-only engines new ScriptRuntime(null) obscured intent; the null just selects the ClassLoader-only resolver. Add a delegating no-arg constructor and use it at the one production call site (create_engine) and all test sites, so a built-ins-only engine reads as new ScriptRuntime(). No behavior change; the resolver-backed two-arg constructor is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Strengthen the throwing-resolver regression from "run reported failure" to "run reported failure AND produced a diagnostic" (result.error truthy), without pinning the internal error wording. Closes the last open GA-cleanup-backlog item. (W-23692110) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
svacas
left a comment
svacas
left a comment
There was a problem hiding this comment.
Findings
P1: Failed engine destruction can leave Java holding a freed resolver context
bridge_finalize_registry() (
data-weave-cli/native-lib/node/src/addon.c
Lines 290 to 338 in 1ca9891
fails, but bridge_finalize() still frees the bridge immediately afterward.
The Java registry then retains a CallbackWeaveResourceResolver whose opaque ctx points to freed memory. A later raw-FFI invocation of that handle can dereference the freed bridge in resolve_module_callback, causing
a use-after-free or crash. The resolver-less case also leaves a supposedly destroyed engine registered.
Do not free the bridge unless registry removal succeeds. Preserve it for retry or surface the destruction failure while retaining valid ownership state.
P2: Completion-sentinel OOM leaves streaming operations permanently pending
Both the streaming (
data-weave-cli/native-lib/node/src/addon.c
Lines 918 to 931 in 1ca9891
(
data-weave-cli/native-lib/node/src/addon.c
Lines 1415 to 1431 in 1ca9891
returning.
No completion is delivered, so:
- The JavaScript promise remains pending forever.
- The thread-safe function is not released.
- Its context points to the freed work record.
- A deferred bridge destruction may finalize from the wrong path while its environment cleanup hook remains registered.
Allocate the completion record before starting the worker, where allocation failure can be reported synchronously, or otherwise ensure the promise and thread-safe-function lifecycle are completed safely.
P2: Resolver tokens leak when isolate acquisition fails
NativeRuntime.initialize() (
data-weave-cli/native-lib/python/src/dataweave/native.py
Lines 232 to 249 in 1ca9891
try block. Resolver tokens are therefore removed only when engine creation fails, not when library loading, ABI binding, isolate creation, or bootstrap detachment fails.
I reproduced this with a library-load failure: the token remained in _resolver_registry, retaining the runtime and resolver indefinitely. Retrying initialization registers another token, accumulating leaked entries.
Include isolate acquisition in the rollback scope and unregister the resolver token on every failed initialization path.
P2: Resolver-backed Python initialization is no longer idempotent
DataWeave.initialize() (
data-weave-cli/native-lib/python/src/dataweave/runtime.py
Lines 43 to 46 in 1ca9891
to the idempotent native initialization method. A second call on an initialized resolver-backed instance therefore raises Cannot install a resolver after initialize().
This regresses the previous behavior, where repeated initialize() calls were harmless. Return early when _native.initialized is already true and add resolver-backed double-initialization coverage.
P2: Raw Node initialization uses an unchecked non-string library path
napi_initialize() (
data-weave-cli/native-lib/node/src/addon.c
Lines 572 to 585 in 1ca9891
napi_get_value_string_utf8. Passing a non-string through the exported raw FFI leaves lib_path unspecified before uv_dlopen uses it.
Other raw entrypoints explicitly validate their conversions, and the PR documents that guarantee. Validate this argument and add it to malformed-inputs.test.ts.
Status
Reviewed head 1ca9891. All macOS, Ubuntu, Windows, SAST, and credential-scanning checks pass. The C source also passes a syntax-only compile; git diff --check reports only a trailing blank line in native.py. I would
request changes based on the findings above.
2 participants
Summary
Replace
native-lib's process-wideScriptRuntimesingleton (one engine, write-once resolver, first-caller-wins) with a handle-keyed registry of per-engineScriptRuntimeobjects living in one shared GraalVM isolate — closing W-23692110 for both the Node and Python bindings, which now drive the same model through the identical*_engineC ABI.DataWeaveinstance (Node or Python) owns an independent native engine — its own module resolver and script cache — addressed by an opaque handle, so multiple instances with different resolvers coexist in one process with no cross-talk.ScriptRuntimesingleton (getInstance()/defaultInstance) and the three legacy singleton C entrypoints (run_script,run_script_callback,run_script_input_output_callback) are removed. All execution is handle-addressed. This is an intentional pre-GA C-ABI break with no shims; dwlib is consumed only by this repo's own bindings, in lockstep.ResolveModuleCallback(thread, ctx, modulePath); resolver dispatch is per-engine via the opaquectx.Two bindings, one model
addon.c) hardened across many review rounds.native.pyimplements a module-level, reference-counted isolate with one engine handle perDataWeaveinstance; the public Python API is unchanged.Design
Consolidated design (both bindings, final state as shipped):
docs/superpowers/specs/2026-08-07-native-lib-multi-engine-design.md. Each Node hardening round and the Python unification are folded into that single doc (provenance map in its appendix).Post-review hardening
The C addon's lifecycle/concurrency and out-of-memory paths were hardened across many Node-binding review rounds — thread-spawn and thread-safe-function failure handling, N-API thread-affinity discipline for resolver bridges, coalesced
cleanup(), a re-init-during-pending-teardown deadlock fix (TEARDOWN_*state machine + live-isolate adoption), atomicg_mutexadmission for all run/stream/transform entrypoints, exhaustivenapi_get_value_*/napi_create_*status checks, OOM-safe setup and worker/callback allocations, and deferral of engine-registry removal until an engine's admitted ops drain.The Python unification then went through the same task-by-task + final-whole-branch review discipline. The final review caught and fixed a cross-thread isolate-teardown hang: the Python glue kept the isolate's bootstrap thread attached for the isolate's life, so a last-release
graal_tear_down_isolaterunning on a different OS thread (e.g. theatexitpath) would block forever. The fix mirrors the Node/Go bindings — detach the bootstrap thread immediately aftergraal_create_isolateand attach a fresh thread on demand for every native call — plus unregistering a resolver token on a failedinitialize().Test plan
./gradlew native-lib:test— Java registry isolation / cross-talk / built-ins-only engines pass; singleton (getInstance()) removed. BUILD SUCCESSFUL../gradlew native-lib:nativeCompile— newcreate_engine*/*_enginesymbols exported; legacy singleton entrypoints removed.native-libNode vitest suite — 952 passed / 32 skipped / 0 failed, incl.independent-engines, teardown-deadlock, and admission regressions plus TCK conformance (676 passed / 0 failed).native-libPython — unit 105 passed; integration 33 passed (incl. multi-instance refcount teardown, per-engine resolver isolation, and a foreign-thread no-hang regression); TCK 729 selected / 0 failed.🤖 Generated with Claude Code