morluto/rea

Fork 0

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

[GH-ISSUE #4752] [Bug] Ghidra snapshot reads omit retained cleanup and completed facts #2477

New issue

Closed

opened 2026-10-17 21:05:22 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

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

Owner

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

Area

Native analysis or decompilation

Observed behavior

A failed final close in GhidraClient.hashTargetSnapshot returns a plain protocol GhidraSessionError with no cleanupFailure, even though the exact descriptor is retained and later client.close fails while retrying it. Completed hash facts are also absent from the returned failure. A separate finalization-cancellation control returns success after the caller aborts during a confirmed descriptor close; that cancellation overlap is already reported as #3977 for DOS readback, so this report concerns retained snapshot-reader ownership/outcomes rather than duplicating #3977.

Reproduction

At 4a076f368c, ran two selected production-client tests in tests/boundary/providers/ghidra/ghidraClient.test.ts using the existing authored fake-Ghidra socket launcher. Both failed, 62 unrelated tests skipped:

reports snapshot descriptor cleanup immediately and resolves its exact error on retry: rejected exact OwnedFileHandle.close; expected cleanupFailure.cleanupIncomplete:true, received undefined. Teardown restores close and releases the client.
preserves completed snapshot hashing and exact cancellation during descriptor close: await actual descriptor close, then abort with an explicit reason; expected typed cancellation, received success. This corroborates the common producer boundary behind #3977.

Small authored Node/socket fixture children and actual private snapshot files/handles ran. No real Ghidra, investigated executable, Java installation, GUI or heavy workload was launched.

Expected behavior

Per docs/tool-design.md#own-the-lifecycle: retained cleanup must be reported immediately and bound to the exact owner through retries; completed observations and physical failures survive close; cancellation reasons and finalization remain explicit. Admitted snapshot reads must settle before their descriptors/root are closed.

Impact and scope

Observed wrong cleanup/result state in one controlled hash-close case and one cancellation case. Ordinary OS failure frequency and real provider incidence: NOT_ENOUGH_INFO. Source-derived additional concern: #withTargetSnapshot tracks handles but not active read operations, and client.close closes those handles before removing roots without joining the reads. That race is not yet reproduced by the two controls above.

REA version / Runtime and platform

Source 4a076f368c, REA6.5.0; Node24.18.0, macOS arm64.

Target and artifact context

Repository-authored fakeGhidra.mjs copied into the actual owned immutable import snapshot and hashed, never treated as an investigated executable.

Interface / Earliest failure stage

Other: production client shared by CLI/MCP / Cleanup, cancellation, or lifecycle.

Relevant output or evidence

Two selected tests failed as above. Source src/ghidra/GhidraClient.ts: #withTargetSnapshot uses a separate raw OwnedFileHandle set, folds close into AggregateError, and infers cancellation from contemporaneous signal.aborted. #performClose retries this set independently of active reads. #snapshotOwners ownership is retained internally; this report does not claim the current code drops the handle or establishes a natural descriptor leak.

Normalize file reads through the existing ArtifactResourceScope/withArtifactRegularFile owner boundary and carry partial observations and exact cleanup capabilities through GhidraSessionError and provider projection. Keep parent/root completion separate from descriptor release. Existing #4502 covers import snapshot creation, #4584 configured artifacts, #3977 final-close cancellation at the DOS consumer, #4506 request lifecycle. Live issue/PR search and retained bodies were reviewed; none covers this snapshot-reader cleanup reporting boundary.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4752 ### Area Native analysis or decompilation ### Observed behavior A failed final close in GhidraClient.hashTargetSnapshot returns a plain protocol GhidraSessionError with no cleanupFailure, even though the exact descriptor is retained and later client.close fails while retrying it. Completed hash facts are also absent from the returned failure. A separate finalization-cancellation control returns success after the caller aborts during a confirmed descriptor close; that cancellation overlap is already reported as #3977 for DOS readback, so this report concerns retained snapshot-reader ownership/outcomes rather than duplicating #3977. ### Reproduction At 4a076f368c62bdfb19415accad342e03a38d1005, ran two selected production-client tests in tests/boundary/providers/ghidra/ghidraClient.test.ts using the existing authored fake-Ghidra socket launcher. Both failed, 62 unrelated tests skipped: - `reports snapshot descriptor cleanup immediately and resolves its exact error on retry`: rejected exact OwnedFileHandle.close; expected cleanupFailure.cleanupIncomplete:true, received undefined. Teardown restores close and releases the client. - `preserves completed snapshot hashing and exact cancellation during descriptor close`: await actual descriptor close, then abort with an explicit reason; expected typed cancellation, received success. This corroborates the common producer boundary behind #3977. Small authored Node/socket fixture children and actual private snapshot files/handles ran. No real Ghidra, investigated executable, Java installation, GUI or heavy workload was launched. ### Expected behavior Per docs/tool-design.md#own-the-lifecycle: retained cleanup must be reported immediately and bound to the exact owner through retries; completed observations and physical failures survive close; cancellation reasons and finalization remain explicit. Admitted snapshot reads must settle before their descriptors/root are closed. ### Impact and scope Observed wrong cleanup/result state in one controlled hash-close case and one cancellation case. Ordinary OS failure frequency and real provider incidence: NOT_ENOUGH_INFO. Source-derived additional concern: #withTargetSnapshot tracks handles but not active read operations, and client.close closes those handles before removing roots without joining the reads. That race is not yet reproduced by the two controls above. ### REA version / Runtime and platform Source 4a076f368c62bdfb19415accad342e03a38d1005, REA6.5.0; Node24.18.0, macOS arm64. ### Target and artifact context Repository-authored fakeGhidra.mjs copied into the actual owned immutable import snapshot and hashed, never treated as an investigated executable. ### Interface / Earliest failure stage Other: production client shared by CLI/MCP / Cleanup, cancellation, or lifecycle. ### Relevant output or evidence Two selected tests failed as above. Source src/ghidra/GhidraClient.ts: #withTargetSnapshot uses a separate raw OwnedFileHandle set, folds close into AggregateError, and infers cancellation from contemporaneous signal.aborted. #performClose retries this set independently of active reads. #snapshotOwners ownership is retained internally; this report does not claim the current code drops the handle or establishes a natural descriptor leak. ### Analysis / Related work Normalize file reads through the existing ArtifactResourceScope/withArtifactRegularFile owner boundary and carry partial observations and exact cleanup capabilities through GhidraSessionError and provider projection. Keep parent/root completion separate from descriptor release. Existing #4502 covers import snapshot creation, #4584 configured artifacts, #3977 final-close cancellation at the DOS consumer, #4506 request lifecycle. Live issue/PR search and retained bodies were reviewed; none covers this snapshot-reader cleanup reporting boundary.

bsakaguchi added the

triage

bug

labels

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

bsakaguchi commented

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

Author

Owner

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

Pushed to main in 5e13e0a87 (reader-owned cancellation hook) and 73965f943 (snapshot lifecycle/outcome fix).

Root cause: snapshot readback kept a separate descriptor set, reported close failure without its retained cleanup capability, inferred cancellation from current signal state, and did not join the logical reader before shutdown. The fix uses the existing artifact scope/file owner, removes the separate set, preserves named completed hash/capture/capacity facts, and projects exact descriptor cleanup through session/provider/CLI/MCP errors. Parent cleanup stays pending while any root remains; live resource labels shrink after individual releases. Physical I/O failure remains primary across a later abort. Hash and bounded capture readers now produce typed cancellation at their own read checkpoints, preserving null/string/Error reasons.

Executed: 22 selected production-client lifecycle checks (60 unrelated tests deselected), 85 resource/session/error/projection/bounded-reader checks across six files, typecheck/lint, runtime builds and commit/pre-push hooks. Includes a real short read before abort, successful/failed descriptor close, shutdown held behind an admitted reader, mixed descriptor/root outcomes, and cleanup retries. The producer checks use actual private files and authored fake-Ghidra Node/socket children. Provider/CLI/SDK MCP projection uses controlled session failures. No real Ghidra, investigated target, heavy CPU test or profiler ran.

Small completed start/hash/close comparison against 4a076f368: identical complete53-token replies; warm medians baseline/candidate elapsed169/174ms, CPU145/153ms, sampled process-family RSS143.4/143.9MB. Separate first processes and two alternating warm repeats; no true cold-cache or statistically established performance claim. Whole-agent/catalog tokens, large inputs, real-provider and failure-path costs remain unmeasured.

Context7 and DeepWiki informed the Node boundary review; primary Node FileHandle documentation describes pending native operations, while the application scope owns completion of the entire reader. This also covers the common producer behind #3977; no complete DOS/public-transport fault qualification is claimed. The separate NativeAOT shared-waiter owner remains outside this fix and under audit.

@morluto commented on GitHub (Oct 17, 2026): Pushed to main in `5e13e0a87` (reader-owned cancellation hook) and `73965f943` (snapshot lifecycle/outcome fix). Root cause: snapshot readback kept a separate descriptor set, reported close failure without its retained cleanup capability, inferred cancellation from current signal state, and did not join the logical reader before shutdown. The fix uses the existing artifact scope/file owner, removes the separate set, preserves named completed hash/capture/capacity facts, and projects exact descriptor cleanup through session/provider/CLI/MCP errors. Parent cleanup stays pending while any root remains; live resource labels shrink after individual releases. Physical I/O failure remains primary across a later abort. Hash and bounded capture readers now produce typed cancellation at their own read checkpoints, preserving null/string/Error reasons. Executed: 22 selected production-client lifecycle checks (60 unrelated tests deselected), 85 resource/session/error/projection/bounded-reader checks across six files, typecheck/lint, runtime builds and commit/pre-push hooks. Includes a real short read before abort, successful/failed descriptor close, shutdown held behind an admitted reader, mixed descriptor/root outcomes, and cleanup retries. The producer checks use actual private files and authored fake-Ghidra Node/socket children. Provider/CLI/SDK MCP projection uses controlled session failures. No real Ghidra, investigated target, heavy CPU test or profiler ran. Small completed start/hash/close comparison against `4a076f368`: identical complete53-token replies; warm medians baseline/candidate elapsed169/174ms, CPU145/153ms, sampled process-family RSS143.4/143.9MB. Separate first processes and two alternating warm repeats; no true cold-cache or statistically established performance claim. Whole-agent/catalog tokens, large inputs, real-provider and failure-path costs remain unmeasured. Context7 and DeepWiki informed the Node boundary review; primary [Node FileHandle documentation](https://nodejs.org/docs/latest-v24.x/api/fs.html#filehandleclose) describes pending native operations, while the application scope owns completion of the entire reader. This also covers the common producer behind #3977; no complete DOS/public-transport fault qualification is claimed. The separate NativeAOT shared-waiter owner remains outside this fix and under audit.

bsakaguchi closed this issue

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

bsakaguchi referenced this issue

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

[PR #2477] [MERGED] fix(browser): exclude sensitive binary lengths from historical captures #5258

bsakaguchi referenced this issue

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

[PR #2491] [MERGED] fix(browser): preserve unknown storage inventory and fingerprint coverage #5269

bsakaguchi referenced this issue

2026-10-31 01:06:48 +00:00

[PR #2872] [CLOSED] fix(captures): redact normalized HAR numeric values #5458