[PR #5281] [MERGED] fix(rizin): accept proven inferior exit after detach #6856

Closed
opened 2026-11-01 21:43:34 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5281
Author: @gensokyo-dev
Created: 10/19/2026
Status: ✅ Merged
Merged: 10/19/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/5277-rizin-inferior-exit-close


📝 Commits (2)

  • efff408 fix(rizin): accept proven inferior exit after detach
  • 6879e1a chore(rizin): integrate latest process lifecycle fixes

📊 Changes

5 files changed (+231 additions, -82 deletions)

View changed files

📝 docs/reverse-engineering-tools.md (+1 -1)
📝 src/process/ownedDebuggerSession.ts (+4 -1)
📝 src/rizin/RizinDebugSessionManager.ts (+56 -68)
➕ tests/boundary/providers/RizinInferiorExit.test.ts (+166 -0)
📝 tests/boundary/providers/RizinInferiorTty.test.ts (+4 -12)

📄 Description

Follow-up: the post-detach ambiguous-readback P1 was corrected in #5290 (909b9f7b6), with a demonstrated failing-before regression, 207 passing checks and actual merged-main Rizin/CLI verification. The validation below records the original normal-workflow checks.

Summary

Fixes #5277. On Linux, starting /usr/bin/true and closing its Rizin session returned cleanup_incomplete when the detached inferior exited before /proc verification; the CLI exited 1 despite complete command Evidence.

Close now succeeds after proving the original identified inferior ended. A live process with mismatched output routing still fails closed and keeps the debugger owned.

Change and scope

Reuse the shared Linux PID/start-time reader; remove Rizin's duplicate /proc/stat parser and its redundant parser tests. Preserve the original identity throughout detach instead of recapturing it. Only consider process-end proof after a correlated native olj read confirms no remaining inferior descriptor. Startup, protocol ambiguity, permissions, PID identity and live-target TTY checks remain required.

Add deterministic exit-during-detach and live-output-redirect regressions using owned process supervision and condition-based synchronization. Document the verified-exit case. No new capability, execution authority, dependency, schema, cache or installer change.

Contract and boundary impact

Owner: Rizin cleanup uses the existing process identity owner. Shared CLI/MCP behavior and existing close result shape are preserved. This is an ordinary bug fix within existing provider boundaries; no new maintainer scope decision is required.

Expected effects (existing workflow):

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.

The patch changes owned-debugger cleanup; real verification launched controlled targets through existing operations.

Evidence and regression coverage

  • npm run test:focused -- 16 Rizin/process files: 206 passed, 1 skipped. The existing optional Rizin 0.7.4 lane was unavailable; the real 0.9.1 checks below ran separately.
  • npm run check and npm run docs:check passed, including architecture guards and five README tests.
  • Actual SDK stdio MCP + official Rizin 0.9.1 Linux x86-64: two baseline first-close failures recovered by explicit retry; two fixed runs closed on the first call. Both revisions preserved stdout/stderr from a live, source-controlled program after debugger close to the selected durable PTY. Actual debug-with-rizin /usr/bin/true dr CLI: exit 1 → 0, complete command Evidence retained.
  • Target digests unchanged; owned process-family cleanup verified; OOM kills 0. CPU1, one heavy job at a time, at most 8 GiB hard limit (public proofs 2 GiB); largest validation peak 4.61 GiB. Counters observed at payload exit with the observer alive, not final whole-scope counters.

Validation performed

ABBA fresh SDK sessions on base 1c2c2f1e7 and tested head 6879e1af8, warm OS file cache: elapsed 4.068–4.474 → 3.867–4.064 s, CPU 3.357–3.833 → 3.220–3.340 s, peak 315.5–319.8 → 320.8–320.9 MiB. This small sample establishes no general speed or memory improvement.

The known-name workflow needed 3 → 2 MCP calls. Recorded request/reply estimates (tiktoken 0.12.0, cl100k_base): 485–495 → 215–218 tokens. No explicit discovery was performed; initialization/protocol/catalog costs and actual model usage are unmeasured. This is lifecycle conformance, not a complex-software investigation benchmark.

Compatibility, safety, and release

No breaking API, package metadata or generated-file change. Linux/Rizin 0.9.1 verified; other engine versions and platforms remain unverified here. Unidentified, unreadable or still-live mismatched processes are not treated as absent. The new fixtures retain ownership and files when cleanup fails.

Author readiness

Human owner: @gensokyo-dev

The owner explicitly authorized ready submission and merging, including CI bypass, in this session. Implementation, technical review and executed validation are recorded above; manual human-review declarations remain for the owner.

  • 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.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 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/5281 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/19/2026 **Status:** ✅ Merged **Merged:** 10/19/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/5277-rizin-inferior-exit-close` --- ### 📝 Commits (2) - [`efff408`](https://github.com/morluto/rea/commit/efff408a19ab6127310be88ae5b814190d46dbb4) fix(rizin): accept proven inferior exit after detach - [`6879e1a`](https://github.com/morluto/rea/commit/6879e1af82ebcbb0c41da02ca9e94530b357f495) chore(rizin): integrate latest process lifecycle fixes ### 📊 Changes **5 files changed** (+231 additions, -82 deletions) <details> <summary>View changed files</summary> 📝 `docs/reverse-engineering-tools.md` (+1 -1) 📝 `src/process/ownedDebuggerSession.ts` (+4 -1) 📝 `src/rizin/RizinDebugSessionManager.ts` (+56 -68) ➕ `tests/boundary/providers/RizinInferiorExit.test.ts` (+166 -0) 📝 `tests/boundary/providers/RizinInferiorTty.test.ts` (+4 -12) </details> ### 📄 Description Follow-up: the post-detach ambiguous-readback P1 was corrected in #5290 (`909b9f7b6`), with a demonstrated failing-before regression, 207 passing checks and actual merged-main Rizin/CLI verification. The validation below records the original normal-workflow checks. ## Summary Fixes #5277. On Linux, starting `/usr/bin/true` and closing its Rizin session returned `cleanup_incomplete` when the detached inferior exited before `/proc` verification; the CLI exited 1 despite complete command Evidence. Close now succeeds after proving the original identified inferior ended. A live process with mismatched output routing still fails closed and keeps the debugger owned. ## Change and scope Reuse the shared Linux PID/start-time reader; remove Rizin's duplicate `/proc/stat` parser and its redundant parser tests. Preserve the original identity throughout detach instead of recapturing it. Only consider process-end proof after a correlated native `olj` read confirms no remaining inferior descriptor. Startup, protocol ambiguity, permissions, PID identity and live-target TTY checks remain required. Add deterministic exit-during-detach and live-output-redirect regressions using owned process supervision and condition-based synchronization. Document the verified-exit case. No new capability, execution authority, dependency, schema, cache or installer change. ## Contract and boundary impact Owner: Rizin cleanup uses the existing process identity owner. Shared CLI/MCP behavior and existing close result shape are preserved. This is an ordinary bug fix within existing provider boundaries; no new maintainer scope decision is required. Expected effects (existing workflow): - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. The patch changes owned-debugger cleanup; real verification launched controlled targets through existing operations. ## Evidence and regression coverage - `npm run test:focused --` 16 Rizin/process files: **206 passed, 1 skipped**. The existing optional Rizin 0.7.4 lane was unavailable; the real 0.9.1 checks below ran separately. - `npm run check` and `npm run docs:check` passed, including architecture guards and five README tests. - Actual SDK stdio MCP + official Rizin 0.9.1 Linux x86-64: two baseline first-close failures recovered by explicit retry; two fixed runs closed on the first call. Both revisions preserved stdout/stderr from a live, source-controlled program after debugger close to the selected durable PTY. Actual `debug-with-rizin /usr/bin/true dr` CLI: exit **1 → 0**, complete command Evidence retained. - Target digests unchanged; owned process-family cleanup verified; OOM kills 0. CPU1, one heavy job at a time, at most 8 GiB hard limit (public proofs 2 GiB); largest validation peak **4.61 GiB**. Counters observed at payload exit with the observer alive, not final whole-scope counters. ## Validation performed ABBA fresh SDK sessions on base `1c2c2f1e7` and tested head `6879e1af8`, warm OS file cache: elapsed **4.068–4.474 → 3.867–4.064 s**, CPU **3.357–3.833 → 3.220–3.340 s**, peak **315.5–319.8 → 320.8–320.9 MiB**. This small sample establishes no general speed or memory improvement. The known-name workflow needed **3 → 2** MCP calls. Recorded request/reply estimates (`tiktoken 0.12.0`, `cl100k_base`): **485–495 → 215–218** tokens. No explicit discovery was performed; initialization/protocol/catalog costs and actual model usage are unmeasured. This is lifecycle conformance, not a complex-software investigation benchmark. ## Compatibility, safety, and release No breaking API, package metadata or generated-file change. Linux/Rizin 0.9.1 verified; other engine versions and platforms remain unverified here. Unidentified, unreadable or still-live mismatched processes are not treated as absent. The new fixtures retain ownership and files when cleanup fails. ## Author readiness Human owner: @gensokyo-dev The owner explicitly authorized ready submission and merging, including CI bypass, in this session. Implementation, technical review and executed validation are recorded above; manual human-review declarations remain for the owner. - [ ] 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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:43:34 +00:00
Sign in to join this conversation.
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#6856
No description provided.