morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4740] [Bug] Native-call preflight loses descriptor cleanup and can outlive client close #2470

New issue

Closed

opened 2026-10-17 21:05:15 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-17 21:05:15 +00:00

Owner

Originally created by @morluto on GitHub (Oct 17, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4740

Before submitting

Searched native-call/digest issues and open native-call PRs. #3185 concerns LLDB pre-launch integrity cleanup; #4723/#4734 concern other native file owners. These preflight descriptor/admission cases are separate.
Only generated inert files and controlled failures/tracers are used.

Area

Evidence, artifacts, or reconstruction

Observed behavior

Two production NativeMacOSProvider controls fail at source 715c9c8a8:

A target-preflight read failure followed by descriptor close failure returns an ordinary error with cleanupIncomplete false; the exact failed descriptor and original read outcome are not retained for client cleanup.
client.close returns successfully while an admitted target-preflight read is gated. The client closes the tracer before the preflight has completed, so later trace dispatch is not joined by that close.

Reproduction

Run the new local boundary controls in tests/boundary/providers/native/nativeCallTargetCleanup.test.ts through the production provider client. They use a real tiny generated file and synthetic Mach-O metadata. The first injects read and close failures on the exact opened FileHandle, then explicitly closes it during teardown. The second gates the actual read and observes client.close settling while the gate remains blocked. Tracer methods are controlled refusal/close stubs; no LLDB or target process is started.

Executed: node node_modules/vitest/vitest.mjs run --project boundary tests/boundary/providers/native/nativeCallTargetCleanup.test.ts. Result: 2 failed; expected cleanupIncomplete true / closed false, both opposite on baseline. macOS arm64, Node24.18.0.

Expected behavior

Use the shared stable-file hash/descriptor owner, retain physical read and cleanup outcomes independently, and retry exact retained capabilities. Client close must stop admission and join the complete admitted call workflow before closing its tracer. Preserve exact cancellation provenance; an aborted signal or Error.name must not reclassify an unrelated physical failure.

Impact and scope

Reproduced provider-boundary lifecycle defects. No ordinary OS close-failure incidence, real LLDB launch-after-close, public CLI/MCP signal timing or Windows reproduction is claimed. NOT_ENOUGH_INFO: external-user frequency. Source-derived additional brittleness: NativeCallObservation has a separate raw open/hash/finally-close implementation and a catch-time signal/AbortError-name heuristic, despite the shared StableFileHash owner.

REA version / target

715c9c8a84; authored inert target bytes with synthetic Mach-O metadata.

Interface / earliest failure stage

Other: production provider boundary / target admission and cleanup lifecycle.

Relevant output or evidence

NativeCallObservation.ts fileSha256 and observeNativeCalls digest catch; NativeMacOSProvider.ts native-call execute branch and client close. Baseline controls fail at cleanupIncomplete and gated-read close-settled assertions. Read/close physical causes and exact handle are retained in the local test harness for diagnosis/teardown.

Proposed checks

Replace duplicate hashing with hashStableFile and an owned resource scope, bind failed descriptor leases and retry errors, and join admitted preflight/trace work before tracer close. Cover late cancellation during close, physical failure with concurrent abort, foreign AbortError/cleanup-marker objects, failed retry admission, target drift, and complete workflow ordering. Retain the existing target-execution authority and selected target inputs.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4740 ### Before submitting - [x] Searched native-call/digest issues and open native-call PRs. #3185 concerns LLDB pre-launch integrity cleanup; #4723/#4734 concern other native file owners. These preflight descriptor/admission cases are separate. - [x] Only generated inert files and controlled failures/tracers are used. ### Area Evidence, artifacts, or reconstruction ### Observed behavior Two production NativeMacOSProvider controls fail at source 715c9c8a8: 1. A target-preflight read failure followed by descriptor close failure returns an ordinary error with cleanupIncomplete false; the exact failed descriptor and original read outcome are not retained for client cleanup. 2. client.close returns successfully while an admitted target-preflight read is gated. The client closes the tracer before the preflight has completed, so later trace dispatch is not joined by that close. ### Reproduction Run the new local boundary controls in tests/boundary/providers/native/nativeCallTargetCleanup.test.ts through the production provider client. They use a real tiny generated file and synthetic Mach-O metadata. The first injects read and close failures on the exact opened FileHandle, then explicitly closes it during teardown. The second gates the actual read and observes client.close settling while the gate remains blocked. Tracer methods are controlled refusal/close stubs; no LLDB or target process is started. Executed: node node_modules/vitest/vitest.mjs run --project boundary tests/boundary/providers/native/nativeCallTargetCleanup.test.ts. Result: 2 failed; expected cleanupIncomplete true / closed false, both opposite on baseline. macOS arm64, Node24.18.0. ### Expected behavior Use the shared stable-file hash/descriptor owner, retain physical read and cleanup outcomes independently, and retry exact retained capabilities. Client close must stop admission and join the complete admitted call workflow before closing its tracer. Preserve exact cancellation provenance; an aborted signal or Error.name must not reclassify an unrelated physical failure. ### Impact and scope Reproduced provider-boundary lifecycle defects. No ordinary OS close-failure incidence, real LLDB launch-after-close, public CLI/MCP signal timing or Windows reproduction is claimed. NOT_ENOUGH_INFO: external-user frequency. Source-derived additional brittleness: NativeCallObservation has a separate raw open/hash/finally-close implementation and a catch-time signal/AbortError-name heuristic, despite the shared StableFileHash owner. ### REA version / target 715c9c8a84e3218c1393212d1a5b8d23fc437d1b; authored inert target bytes with synthetic Mach-O metadata. ### Interface / earliest failure stage Other: production provider boundary / target admission and cleanup lifecycle. ### Relevant output or evidence NativeCallObservation.ts fileSha256 and observeNativeCalls digest catch; NativeMacOSProvider.ts native-call execute branch and client close. Baseline controls fail at cleanupIncomplete and gated-read close-settled assertions. Read/close physical causes and exact handle are retained in the local test harness for diagnosis/teardown. ### Proposed checks Replace duplicate hashing with hashStableFile and an owned resource scope, bind failed descriptor leases and retry errors, and join admitted preflight/trace work before tracer close. Cover late cancellation during close, physical failure with concurrent abort, foreign AbortError/cleanup-marker objects, failed retry admission, target drift, and complete workflow ordering. Retain the existing target-execution authority and selected target inputs.

bsakaguchi added the

bug

label

2026-10-17 21:05:15 +00:00

bsakaguchi commented

2026-10-17 21:05:18 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 17, 2026):

Confirmed reproduction of two lifecycle defects: the preflight read/close double-failure collapses into a plain error with cleanupIncomplete:false, so the exact failed descriptor lease is lost; and client.close settling while the preflight read is still gated means the admitted work is never joined, which can leave later trace dispatch racing a closed tracer. Aligning with the #4723/#4734 fix direction (shared stable-file owner + owned resource scope, retry exact retained leases) looks right. One suggestion: make the close-join contract explicit in the returned error state — e.g. report admitted-but-not-joined preflight work as retained cleanup with the gated read outcome attached — so clients can distinguish "joined and completed" from "admission stopped but work still outstanding" instead of inferring it from tracer state.

@XinwangTao commented on GitHub (Oct 17, 2026): Confirmed reproduction of two lifecycle defects: the preflight read/close double-failure collapses into a plain error with cleanupIncomplete:false, so the exact failed descriptor lease is lost; and client.close settling while the preflight read is still gated means the admitted work is never joined, which can leave later trace dispatch racing a closed tracer. Aligning with the #4723/#4734 fix direction (shared stable-file owner + owned resource scope, retry exact retained leases) looks right. One suggestion: make the close-join contract explicit in the returned error state — e.g. report admitted-but-not-joined preflight work as retained cleanup with the gated read outcome attached — so clients can distinguish "joined and completed" from "admission stopped but work still outstanding" instead of inferring it from tracer state.

bsakaguchi commented

2026-10-17 21:05:18 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Published in 69e670d268 on main, rebased over the independent Electron test-helper update; all six audited source/test hashes were unchanged.

Replaced the duplicate raw fileSha256 loop and injectable digest shortcut with the shared hashStableFile owner. NativeCallTarget translates physical hash/cancellation/cleanup outcomes once and retains the exact descriptor lease. NativeMacOSClient owns a dedicated call scope: close stops admission, joins admitted preflight/trace work, retries retained file cleanup, then closes the tracer. Failed retry admission blocks replacement reads/trace dispatch. Removed the AbortError-name and catch-time signal-state heuristics. Exact caller reasons and physical read failures survive; cancellation with a completed tracer result retains its facts in the existing native-call partial-observation model. Unknown thrown tracer objects confer no descriptor authority.

Baseline: two production-client controls failed (lost cleanup and close settling while the read gate remained blocked). Final and post-rebase: 81 checks passed in seven files, including six target cleanup controls, five real-file cancellation/physical-error controls, nine call-projection controls, three MCP transport controls, 23 shared hash ownership checks, and plist/signature overlap. Static checks and commit/push hooks passed. Generated inert files and controlled tracers/faults only; no LLDB or target process was launched. No public CLI/MCP signal-timing, real OS close-failure incidence or Windows qualification.

Small completed provider workflow comparison: six byte-identical complete AnalysisExecution replies, 959 raw cl100k_base tokens. Real 65,549-byte target hashing, controlled completed trace, production projection and client close. First-process baseline/candidate: elapsed 0.286/0.311 s, CPU 0.444/0.453 s, sampled process-family RSS 256,098,304/255,721,472 bytes. Two warm-repeat medians: elapsed 0.286/0.284 s, CPU 0.440/0.445 s, RSS 261,423,104/257,777,664 bytes. No performance improvement claim; whole-agent/catalog, live LLDB/target/CLI, large-file and injected-failure costs remain unmeasured. Owned comparison worktree verified and removed normally. Heavy CPU profiling/stress remained stopped.

The API interpretation reuses the Node24 explicit-close/arbitrary-abort-reason research checked through Context7/DeepWiki in #4735, together with the actual StableFileHash/OwnedRegularFile implementation and controls. This does not establish loaded-image digest identity; existing native-call limitations remain applicable.

@morluto commented on GitHub (Oct 17, 2026): Published in 69e670d268cb446cf8080a936836393ada57d223 on main, rebased over the independent Electron test-helper update; all six audited source/test hashes were unchanged. Replaced the duplicate raw fileSha256 loop and injectable digest shortcut with the shared hashStableFile owner. NativeCallTarget translates physical hash/cancellation/cleanup outcomes once and retains the exact descriptor lease. NativeMacOSClient owns a dedicated call scope: close stops admission, joins admitted preflight/trace work, retries retained file cleanup, then closes the tracer. Failed retry admission blocks replacement reads/trace dispatch. Removed the AbortError-name and catch-time signal-state heuristics. Exact caller reasons and physical read failures survive; cancellation with a completed tracer result retains its facts in the existing native-call partial-observation model. Unknown thrown tracer objects confer no descriptor authority. Baseline: two production-client controls failed (lost cleanup and close settling while the read gate remained blocked). Final and post-rebase: 81 checks passed in seven files, including six target cleanup controls, five real-file cancellation/physical-error controls, nine call-projection controls, three MCP transport controls, 23 shared hash ownership checks, and plist/signature overlap. Static checks and commit/push hooks passed. Generated inert files and controlled tracers/faults only; no LLDB or target process was launched. No public CLI/MCP signal-timing, real OS close-failure incidence or Windows qualification. Small completed provider workflow comparison: six byte-identical complete AnalysisExecution replies, 959 raw cl100k_base tokens. Real 65,549-byte target hashing, controlled completed trace, production projection and client close. First-process baseline/candidate: elapsed 0.286/0.311 s, CPU 0.444/0.453 s, sampled process-family RSS 256,098,304/255,721,472 bytes. Two warm-repeat medians: elapsed 0.286/0.284 s, CPU 0.440/0.445 s, RSS 261,423,104/257,777,664 bytes. No performance improvement claim; whole-agent/catalog, live LLDB/target/CLI, large-file and injected-failure costs remain unmeasured. Owned comparison worktree verified and removed normally. Heavy CPU profiling/stress remained stopped. The API interpretation reuses the Node24 explicit-close/arbitrary-abort-reason research checked through Context7/DeepWiki in #4735, together with the actual StableFileHash/OwnedRegularFile implementation and controls. This does not establish loaded-image digest identity; existing native-call limitations remain applicable.

bsakaguchi closed this issue

2026-10-31 00:34:18 +00:00

bsakaguchi commented

2026-10-31 00:34:20 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 18, 2026):

69e670d covers the two lifecycle defects well — close now stops admission, joins admitted preflight/trace work, retries retained cleanup, then closes the tracer, which closes the race I hit. One remaining question on the close-join contract: does the returned error state still report admitted-but-not-joined preflight work as retained cleanup with the gate reason? That was the piece that made the failure auditable in my repro — if it's been dropped along with the AbortError heuristics, keeping a small note in the error payload would preserve the diagnosis path. Otherwise this looks ready to close.

@XinwangTao commented on GitHub (Oct 18, 2026): 69e670d covers the two lifecycle defects well — close now stops admission, joins admitted preflight/trace work, retries retained cleanup, then closes the tracer, which closes the race I hit. One remaining question on the close-join contract: does the returned error state still report admitted-but-not-joined preflight work as retained cleanup with the gate reason? That was the piece that made the failure auditable in my repro — if it's been dropped along with the AbortError heuristics, keeping a small note in the error payload would preserve the diagnosis path. Otherwise this looks ready to close.

bsakaguchi referenced this issue

2026-10-31 00:48:18 +00:00

[GH-ISSUE #7075] Windows: Ghidra provider fails to start - LaunchSupport cannot write java_home.save (access denied) #4264

bsakaguchi referenced this issue

2026-10-31 00:48:19 +00:00

[GH-ISSUE #7075] Windows: Ghidra provider fails to start - LaunchSupport cannot write java_home.save (access denied) #4264

bsakaguchi referenced this issue

2026-10-31 01:05:10 +00:00

[PR #2470] [MERGED] fix(ghidra): pre-create the launch-support config directory #5253

bsakaguchi referenced this issue

2026-10-31 01:05:42 +00:00

[PR #2694] fix(wasm): raise the WABT output budget to 128 MiB #5318