morluto/rea

Fork 0

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

[PR #1959] feat(process): add owned debugger shutdown lifecycle #4934

New issue

Open

opened 2026-10-31 00:57:07 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:57:07 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1959
Author: @dandgabr
Created: 10/11/2026
Status: 🔄 Open

Base: main ← Head: feat/owned-debugger-process-lifecycle

📝 Commits (10+)

eada258 feat(process): add owned debugger shutdown lifecycle
098a548 fix(process): preserve debugger shutdown ownership
2667a65 test(process): cover debugger shutdown retries
6a68979 fix(process): unref inherited debugger output pipes
4b67b60 fix(process): retain debugger ownership until safe shutdown
310109f fix(process): preserve replacement debugger sessions
3bc2fc5 fix(process): retain exited debugger until output settles
03ac0b6 Merge commit '538f8cc7' into feat/owned-debugger-process-lifecycle
064297f fix(process): retain debugger ownership after detach failure
011344b fix(process): cap debugger output after exit

📊 Changes

35 files changed (+5496 additions, -393 deletions)

View changed files

📝 docs/reverse-engineering-tools.md (+2 -2)
📝 src/android/JadxDoctor.ts (+4 -2)
📝 src/application/AnalysisContextQueries.test.ts (+1 -1)
📝 src/application/binary/BinarySessionOpen.ts (+1 -1)
📝 src/application/native/NativeFunctionMatchingService.ts (+4 -2)
📝 src/browser/CdpEndpoint.ts (+5 -1)
📝 src/browser/WebBundleAnalysisWorker.test.ts (+3 -2)
📝 src/browser/history/NodeCaptureAdapter.ts (+1 -1)
📝 src/cli/reverseEngineeringCommands.ts (+5 -3)
📝 src/cliJsonPaths.test.ts (+3 -1)
📝 src/contracts/gdbToolContracts.ts (+3 -3)
📝 src/contracts/rizinDebugToolContracts.ts (+1 -1)
📝 src/domain/analysisErrorBase.ts (+1 -0)
📝 src/domain/analysisErrorCleanup.test.ts (+13 -0)
📝 src/gdb/GdbSessionManager.ts (+212 -13)
📝 src/ida/IdaOperationFailures.test.ts (+2 -2)
📝 src/pcap/PcapSnapshot.ts (+1 -1)
➕ src/process/inferiorInputSink.identityRace.test.ts (+67 -0)
➕ src/process/inferiorInputSink.test.ts (+109 -0)
➕ src/process/inferiorInputSink.ts (+99 -0)

...and 15 more files

📄 Description

Summary

Owned debugger shutdown now shares each full stop attempt, requires verified independent inferior stdin/stdout/stderr before detaching and signalling a POSIX debugger, and retains sessions until process and inherited-output settlement are confirmed. Windows Job Object launches fail closed for direct-child termination; failed native observation continues with referenced, capped-backoff polling, and explicit cleanup promptly resumes polling and drains newly blocked output.

GDB and Rizin verify inferior input through Linux /proc, preserve caller-selected REA_PROCESS_RUN_ID values instead of leaking REA's ownership token, and fail closed on unknown descriptor state. The latest correction requires an explicit GDB input classification: an injected verifier returning no value previously became durable; it now rejects before detach or signalling. Fixtures have bounded lifetimes and ownership-based teardown. Existing CLI load budgets are retained.

This updates existing supervision and debugger consumers; no new provider, tool, execution authority, or mutation gate is introduced. It continues the lifecycle prerequisite split from #1279. Maintainer scope judgment remains required; no new approval is claimed.

Validation (Linux, Node 24.18.0 / npm 11.16.0)

Merged upstream/main through 9af0b6113 in d1c6619de, following the earlier 41aa15462 synchronization. The branch preserves both its CLI timeout and upstream's uninstall regression. GDB input-proof fix: 202d3f7c7.

New GDB regression: before correction, 151 passed / 1 failed because unconfirmed input incorrectly allowed successful close; afterward all 152 passed.
Focused batch: GdbSessionManagerCoverage, RizinInferiorTty, clientProfileLifecycle, nativeDemangleCli — 182 passed, 1 pre-existing macOS-only skip.
Focused batch: GdbSessionReviewRegressions, GdbSessionManagers, GdbCommandQueue, ownedDebuggerSession, providerProcessOutput, WindowsOwnedProcess, inferiorInputSink, RizinDebugSessionManagers, RizinInferiorEnvironment — 99 passed.
Latest upstream-sync verification at d1c6619de: 10 focused files passed, 299 tests passed / 4 platform-gated tests skipped, covering debugger lifecycle, GDB coverage, error projection/cleanup and upstream snapshot-handle cleanup through compiled CLI/MCP. npm run check:fast — typecheck and lint passed freshly and again from the pre-push hook; git diff --check passed.
Follow-up verification at 049603306: all 43 ownedDebuggerSession tests passed; npm run build:cached, npm run check:fast, and npm run docs:check passed. The actual rebuilt rea debug-with-rizin --help output explicitly includes stdin and warns that the inferior may consume terminal input. Checks used the pinned Node/npm above.
Real Rizin 0.7.4 TTY regression executed (not skipped): verified detach, target survival and output after debugger close.
Separate production-boundary probe using installed GDB 17.2 and the built GdbSessionManager: a source-owned Python inferior stopped at its own SIGTRAP, detached, survived debugger close, and wrote both stdout/stderr markers to its selected TTY. Fixture resources were cleaned up. Earlier exploratory probes used MI syntax through the console API or waited for a naturally exiting target and did not establish survival; the corrected probe above passed.

The prior 40 review threads were read alongside paginated comments/reviews. Two subsequent findings are corrected in 6ef308f3d and 049603306: Rizin CLI help now explicitly discloses inferior stdin and possible terminal-input consumption; debugger fixture setup prepares the ownership reader before launching fixtures, with the configured Swift compiler budget plus hook overhead. Full ownership-based teardown and its assertions are retained. Independent source review of these two follow-up commits found no actionable issue.

Limits and compatibility

No Windows or macOS runtime was executed here; the cold Swift compile/setup path is source-reviewed but was not reproduced on macOS; Windows tests use native-binding doubles and do not prove Job Object behavior on Windows. No full suite, package-install verification, or public CLI/MCP GDB end-to-end workflow was run. The GDB smoke establishes its production manager boundary only. Unknown detach, input, output, process identity, or settlement retains ownership rather than claiming cleanup. No performance improvement is claimed.

Author readiness

Human owner: @dandgabr

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1959 **Author:** [@dandgabr](https://github.com/dandgabr) **Created:** 10/11/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/owned-debugger-process-lifecycle` --- ### 📝 Commits (10+) - [`eada258`](https://github.com/morluto/rea/commit/eada258ea3145832a0ac0979739125b7e284ccab) feat(process): add owned debugger shutdown lifecycle - [`098a548`](https://github.com/morluto/rea/commit/098a548d9f7af81acd92fb02f40444dad72c7eda) fix(process): preserve debugger shutdown ownership - [`2667a65`](https://github.com/morluto/rea/commit/2667a65daf28862d6b82791c4770ab70b4656b3c) test(process): cover debugger shutdown retries - [`6a68979`](https://github.com/morluto/rea/commit/6a68979e65c59f0395e1d57e06fbf0435862008c) fix(process): unref inherited debugger output pipes - [`4b67b60`](https://github.com/morluto/rea/commit/4b67b6045b54d334f2f61efe37a7a47a4e633b74) fix(process): retain debugger ownership until safe shutdown - [`310109f`](https://github.com/morluto/rea/commit/310109f54d2fb1437840af9053d29ced5c5d1e97) fix(process): preserve replacement debugger sessions - [`3bc2fc5`](https://github.com/morluto/rea/commit/3bc2fc53da14bf3dc46609fb7cd3300d0ed85fc4) fix(process): retain exited debugger until output settles - [`03ac0b6`](https://github.com/morluto/rea/commit/03ac0b6257ed562cec5aced87fb2913023580f43) Merge commit '538f8cc7' into feat/owned-debugger-process-lifecycle - [`064297f`](https://github.com/morluto/rea/commit/064297f54ff58def11fa08ad87cbf8796233f55a) fix(process): retain debugger ownership after detach failure - [`011344b`](https://github.com/morluto/rea/commit/011344b877891846233d20291d6d4352893e2ff8) fix(process): cap debugger output after exit ### 📊 Changes **35 files changed** (+5496 additions, -393 deletions) <details> <summary>View changed files</summary> 📝 `docs/reverse-engineering-tools.md` (+2 -2) 📝 `src/android/JadxDoctor.ts` (+4 -2) 📝 `src/application/AnalysisContextQueries.test.ts` (+1 -1) 📝 `src/application/binary/BinarySessionOpen.ts` (+1 -1) 📝 `src/application/native/NativeFunctionMatchingService.ts` (+4 -2) 📝 `src/browser/CdpEndpoint.ts` (+5 -1) 📝 `src/browser/WebBundleAnalysisWorker.test.ts` (+3 -2) 📝 `src/browser/history/NodeCaptureAdapter.ts` (+1 -1) 📝 `src/cli/reverseEngineeringCommands.ts` (+5 -3) 📝 `src/cliJsonPaths.test.ts` (+3 -1) 📝 `src/contracts/gdbToolContracts.ts` (+3 -3) 📝 `src/contracts/rizinDebugToolContracts.ts` (+1 -1) 📝 `src/domain/analysisErrorBase.ts` (+1 -0) 📝 `src/domain/analysisErrorCleanup.test.ts` (+13 -0) 📝 `src/gdb/GdbSessionManager.ts` (+212 -13) 📝 `src/ida/IdaOperationFailures.test.ts` (+2 -2) 📝 `src/pcap/PcapSnapshot.ts` (+1 -1) ➕ `src/process/inferiorInputSink.identityRace.test.ts` (+67 -0) ➕ `src/process/inferiorInputSink.test.ts` (+109 -0) ➕ `src/process/inferiorInputSink.ts` (+99 -0) _...and 15 more files_ </details> ### 📄 Description ## Summary Owned debugger shutdown now shares each full stop attempt, requires verified independent inferior stdin/stdout/stderr before detaching and signalling a POSIX debugger, and retains sessions until process and inherited-output settlement are confirmed. Windows Job Object launches fail closed for direct-child termination; failed native observation continues with referenced, capped-backoff polling, and explicit cleanup promptly resumes polling and drains newly blocked output. GDB and Rizin verify inferior input through Linux `/proc`, preserve caller-selected `REA_PROCESS_RUN_ID` values instead of leaking REA's ownership token, and fail closed on unknown descriptor state. The latest correction requires an explicit GDB input classification: an injected verifier returning no value previously became `durable`; it now rejects before detach or signalling. Fixtures have bounded lifetimes and ownership-based teardown. Existing CLI load budgets are retained. This updates existing supervision and debugger consumers; no new provider, tool, execution authority, or mutation gate is introduced. It continues the lifecycle prerequisite split from #1279. Maintainer scope judgment remains required; no new approval is claimed. ## Validation (Linux, Node 24.18.0 / npm 11.16.0) Merged `upstream/main` through `9af0b6113` in `d1c6619de`, following the earlier `41aa15462` synchronization. The branch preserves both its CLI timeout and upstream's uninstall regression. GDB input-proof fix: `202d3f7c7`. - New GDB regression: before correction, 151 passed / 1 failed because unconfirmed input incorrectly allowed successful close; afterward all 152 passed. - Focused batch: `GdbSessionManagerCoverage`, `RizinInferiorTty`, `clientProfileLifecycle`, `nativeDemangleCli` — 182 passed, 1 pre-existing macOS-only skip. - Focused batch: `GdbSessionReviewRegressions`, `GdbSessionManagers`, `GdbCommandQueue`, `ownedDebuggerSession`, `providerProcessOutput`, `WindowsOwnedProcess`, `inferiorInputSink`, `RizinDebugSessionManagers`, `RizinInferiorEnvironment` — 99 passed. - Latest upstream-sync verification at `d1c6619de`: 10 focused files passed, 299 tests passed / 4 platform-gated tests skipped, covering debugger lifecycle, GDB coverage, error projection/cleanup and upstream snapshot-handle cleanup through compiled CLI/MCP. `npm run check:fast` — typecheck and lint passed freshly and again from the pre-push hook; `git diff --check` passed. - Follow-up verification at `049603306`: all 43 `ownedDebuggerSession` tests passed; `npm run build:cached`, `npm run check:fast`, and `npm run docs:check` passed. The actual rebuilt `rea debug-with-rizin --help` output explicitly includes stdin and warns that the inferior may consume terminal input. Checks used the pinned Node/npm above. - Real Rizin 0.7.4 TTY regression executed (not skipped): verified detach, target survival and output after debugger close. - Separate production-boundary probe using installed GDB 17.2 and the built `GdbSessionManager`: a source-owned Python inferior stopped at its own SIGTRAP, detached, survived debugger close, and wrote both stdout/stderr markers to its selected TTY. Fixture resources were cleaned up. Earlier exploratory probes used MI syntax through the console API or waited for a naturally exiting target and did not establish survival; the corrected probe above passed. The prior 40 review threads were read alongside paginated comments/reviews. Two subsequent findings are corrected in `6ef308f3d` and `049603306`: Rizin CLI help now explicitly discloses inferior stdin and possible terminal-input consumption; debugger fixture setup prepares the ownership reader before launching fixtures, with the configured Swift compiler budget plus hook overhead. Full ownership-based teardown and its assertions are retained. Independent source review of these two follow-up commits found no actionable issue. ## Limits and compatibility No Windows or macOS runtime was executed here; the cold Swift compile/setup path is source-reviewed but was not reproduced on macOS; Windows tests use native-binding doubles and do not prove Job Object behavior on Windows. No full suite, package-install verification, or public CLI/MCP GDB end-to-end workflow was run. The GDB smoke establishes its production manager boundary only. Unknown detach, input, output, process identity, or settlement retains ownership rather than claiming cleanup. No performance improvement is claimed. ## Author readiness Human owner: @dandgabr - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 00:57:07 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#4934

No description provided.

Rows
Columns