morluto/rea

Fork 0

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

[GH-ISSUE #5340] [Feature] Optional per-function Ghidra decompile deadline that fails one request instead of closing the session #3248

New issue

Closed

opened 2026-10-31 00:38:21 +00:00 by bsakaguchi · 6 comments

bsakaguchi commented

2026-10-31 00:38:21 +00:00

Owner

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

Before submitting

I searched the open and closed issues and pull requests for a related proposal.
I understand the contribution scope must be reviewed before integration; open questions are welcome here.
This request does not include credentials, private samples, or sensitive target data.

Area

Native analysis or decompilation

Current limitation

The Ghidra bridge decompiles with no timeout: decompiler.decompileFunction(function, 0, monitor) at bridge/ghidra/ReaGhidraBridge.java:3327-3331. initializeDecompiler (:916-927) sets no time limit either. The current documentation matches: "Native decompilation has no fixed per-function deadline" (docs/ghidra-provider.md:165).

Requests go through a FIFO, one at a time (docs/ghidra-provider.md:166), so a single pathological function holds up every later request on that program. That includes list_procedures, xrefs and the rest of a batch_decompile. The caller's only way out is to cancel the active request. That closes the entire Ghidra session (src/ghidra/GhidraRequestQueue.ts:126-131, "Ghidra session closed after active request cancellation"), so the next call re-imports the binary and re-runs auto-analysis from scratch, which can take hours on a large app.

What I checked: I drove GhidraRequestQueue directly at 833136507 with no Ghidra, using a slow request that a caller aborts after 20 ms:

[C2] cancelled active request -> cancelled: x
[C2] unrelated queued request -> process: Ghidra session closed after active request cancellation
[C2] later request on same queue -> process: Ghidra session closed after active request cancellation
[C2] onActiveCancellation (provider close) invoked: 1

The unbounded decompile is source-derived (ReaGhidraBridge.java:3327). I did not reproduce it with a pathological function, because Ghidra is not installed here. Cancellation discarding the session is documented (docs/native-investigation.md:77-79). This request is about having a way to recover that doesn't involve cancelling.

Use cases

Large binaries where one decompile never finishes: function listing and reopening queue behind it and time out.
batch_decompile and "export all pseudocode" over thousands of functions, where one bad function should yield one per-function error rather than block the batch.

Desired outcome and success criteria

A configuration such as REA_GHIDRA_DECOMPILE_TIMEOUT_MS, validated like REA_JADX_OPERATION_TIMEOUT_MS (src/android/JadxRelease.ts:26-30). Unset keeps today's unbounded behavior.
When it is set and expires, Ghidra's DecompileResults.isTimedOut() produces a typed timeout error for that request only. That error should:
- be marked retryable;
- carry remediation text that says how to raise the limit, not "run rea doctor";
- leave the session open; Ghidra restarts its decompiler process on the next call.
In analyze_function and batch_decompile, a timed-out function keeps its non-decompiler facts and reports the timeout as a per-function limitation (this ties into #4644 / #4650).

Success criterion: with a deadline of a few milliseconds on a fixture, procedure_pseudo_code returns the typed timeout, and the next list_procedures on the same session succeeds without a new import.

Expected interaction with targets

Read existing files or captured data

Target and scope details (optional)

Ghidra provider only. Non-goal: a fixed default deadline. #5325 describes a legitimate decompile that took about 600 seconds, so a hard-coded 30-second cap would break real work.

Verification available so far

I have a prototype or observed results

Verification details (optional)

Prior art:

Zfinix/rea commit 140c71240 caps decompiles at 30 s, checks isTimedOut(), and throws decompile_timeout.
Open PR #4439 contains the same cap among about 45 other changed files.

Gap in both: a bridge decompile_timeout reaches MCP callers as a generic execution_failure with "Run rea doctor…" advice. I found this with a source-level probe of the TypeScript projection; a real Ghidra run was not available. So a TypeScript mapping is needed as well as the Java change.

Alternatives considered

Hard-coded 30 s (fork, #4439): too short for real large functions (#5325) and not configurable.
Caller-side timeouts: cancelling closes the whole session (see above).
Background jobs (#5271): complementary. Jobs avoid blocking the MCP call but still occupy the single Program queue.

Possible direction

Pass the configured seconds to decompileFunction. Map isTimedOut() to a new bridge failure code. Project that code in the Ghidra session layer to the shared timeout error. Add entries to docs/error-contract.schema.json / docs/user-visible-errors.csv if the code is new.

References

PR #4439 (open, conflicting), PR #5325 (payload cap), #4644 / PR #4650 (partial dossiers), #5271 (background jobs), #5032 (oversized pseudocode).

Originally created by @coygeek on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5340 ### Before submitting - [x] I searched the open and closed issues and pull requests for a related proposal. - [x] I understand the contribution scope must be reviewed before integration; open questions are welcome here. - [x] This request does not include credentials, private samples, or sensitive target data. ### Area Native analysis or decompilation ### Current limitation The Ghidra bridge decompiles with no timeout: `decompiler.decompileFunction(function, 0, monitor)` at `bridge/ghidra/ReaGhidraBridge.java:3327-3331`. `initializeDecompiler` (`:916-927`) sets no time limit either. The current documentation matches: "Native decompilation has no fixed per-function deadline" (`docs/ghidra-provider.md:165`). Requests go through a FIFO, one at a time (`docs/ghidra-provider.md:166`), so a single pathological function holds up every later request on that program. That includes `list_procedures`, `xrefs` and the rest of a `batch_decompile`. The caller's only way out is to cancel the active request. That closes the entire Ghidra session (`src/ghidra/GhidraRequestQueue.ts:126-131`, "Ghidra session closed after active request cancellation"), so the next call re-imports the binary and re-runs auto-analysis from scratch, which can take hours on a large app. What I checked: I drove `GhidraRequestQueue` directly at `833136507` with no Ghidra, using a slow request that a caller aborts after 20 ms: ``` [C2] cancelled active request -> cancelled: x [C2] unrelated queued request -> process: Ghidra session closed after active request cancellation [C2] later request on same queue -> process: Ghidra session closed after active request cancellation [C2] onActiveCancellation (provider close) invoked: 1 ``` The unbounded decompile is source-derived (`ReaGhidraBridge.java:3327`). I did not reproduce it with a pathological function, because Ghidra is not installed here. Cancellation discarding the session is documented (`docs/native-investigation.md:77-79`). This request is about having a way to recover that doesn't involve cancelling. ### Use cases - Large binaries where one decompile never finishes: function listing and reopening queue behind it and time out. - `batch_decompile` and "export all pseudocode" over thousands of functions, where one bad function should yield one per-function error rather than block the batch. ### Desired outcome and success criteria - A configuration such as `REA_GHIDRA_DECOMPILE_TIMEOUT_MS`, validated like `REA_JADX_OPERATION_TIMEOUT_MS` (`src/android/JadxRelease.ts:26-30`). Unset keeps today's unbounded behavior. - When it is set and expires, Ghidra's `DecompileResults.isTimedOut()` produces a typed timeout error for that request only. That error should: - be marked retryable; - carry remediation text that says how to raise the limit, not "run rea doctor"; - leave the session open; Ghidra restarts its decompiler process on the next call. - In `analyze_function` and `batch_decompile`, a timed-out function keeps its non-decompiler facts and reports the timeout as a per-function limitation (this ties into #4644 / #4650). Success criterion: with a deadline of a few milliseconds on a fixture, `procedure_pseudo_code` returns the typed timeout, and the next `list_procedures` on the same session succeeds without a new import. ### Expected interaction with targets Read existing files or captured data ### Target and scope details (optional) Ghidra provider only. Non-goal: a fixed default deadline. #5325 describes a legitimate decompile that took about 600 seconds, so a hard-coded 30-second cap would break real work. ### Verification available so far I have a prototype or observed results ### Verification details (optional) Prior art: - Zfinix/rea commit `140c71240` caps decompiles at 30 s, checks `isTimedOut()`, and throws `decompile_timeout`. - Open PR #4439 contains the same cap among about 45 other changed files. Gap in both: a bridge `decompile_timeout` reaches MCP callers as a generic `execution_failure` with "Run `rea doctor`…" advice. I found this with a source-level probe of the TypeScript projection; a real Ghidra run was not available. So a TypeScript mapping is needed as well as the Java change. ### Alternatives considered - Hard-coded 30 s (fork, #4439): too short for real large functions (#5325) and not configurable. - Caller-side timeouts: cancelling closes the whole session (see above). - Background jobs (#5271): complementary. Jobs avoid blocking the MCP call but still occupy the single Program queue. ### Possible direction Pass the configured seconds to `decompileFunction`. Map `isTimedOut()` to a new bridge failure code. Project that code in the Ghidra session layer to the shared timeout error. Add entries to `docs/error-contract.schema.json` / `docs/user-visible-errors.csv` if the code is new. ### References - PR #4439 (open, conflicting), PR #5325 (payload cap), #4644 / PR #4650 (partial dossiers), #5271 (background jobs), #5032 (oversized pseudocode).

bsakaguchi closed this issue

2026-10-31 00:38:21 +00:00

bsakaguchi commented

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

Author

Owner

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

赞同这个方向。FIFO 队列 + 无超时反编译意味着单个病态函数能阻塞同一 program 的所有后续请求，而唯一逃生通道（取消）会关闭整个会话并触发重导 + 重跑 auto-analysis，对大型二进制代价极高，这个痛点真实。几个建议：1）「默认不设限、可配置」很关键，赞成不硬编码 30s——#5325 里 600 秒的合法反编译说明固定 cap 会误伤真实工作负载；2）超时错误标记 retryable 并带「如何调大 limit」的 remediation，比 generic execution_failure 好太多，但注意 #4439 的同一 cap 实现里 isTimedOut 映射到 execution_failure 正是 gap 所在，TS 层映射要一起做，否则 Java 侧改了 MCP 调用方仍然看不到类型化错误；3）与 #4650 的 partial dossier 结合很好：超时函数保留非反编译 facts + 标记 per-function limitation，这样 batch_decompile 的「一个坏函数不毁整批」才真正成立；4）建议 deadline 语义按 request 而非按 program 粒度透传（配置可默认，但调用方应能按次覆盖），因为单个会话里既可能碰到巨型函数需要放宽、也可能需要快速失败探测；5）#5271 的 background jobs 是互补的，但如你所说它只解决阻塞 MCP 调用、不解决占用队列，短期还是以超时机制为主更务实。补充：文档 ghidra-provider.md:165-166 提到 FIFO 和 no deadline 的说明也要随实现更新，避免文档与实际行为再次脱节。

@XinwangTao commented on GitHub (Oct 20, 2026): 赞同这个方向。FIFO 队列 + 无超时反编译意味着单个病态函数能阻塞同一 program 的所有后续请求，而唯一逃生通道（取消）会关闭整个会话并触发重导 + 重跑 auto-analysis，对大型二进制代价极高，这个痛点真实。几个建议：1）「默认不设限、可配置」很关键，赞成不硬编码 30s——#5325 里 600 秒的合法反编译说明固定 cap 会误伤真实工作负载；2）超时错误标记 retryable 并带「如何调大 limit」的 remediation，比 generic execution_failure 好太多，但注意 #4439 的同一 cap 实现里 isTimedOut 映射到 execution_failure 正是 gap 所在，TS 层映射要一起做，否则 Java 侧改了 MCP 调用方仍然看不到类型化错误；3）与 #4650 的 partial dossier 结合很好：超时函数保留非反编译 facts + 标记 per-function limitation，这样 batch_decompile 的「一个坏函数不毁整批」才真正成立；4）建议 deadline 语义按 request 而非按 program 粒度透传（配置可默认，但调用方应能按次覆盖），因为单个会话里既可能碰到巨型函数需要放宽、也可能需要快速失败探测；5）#5271 的 background jobs 是互补的，但如你所说它只解决阻塞 MCP 调用、不解决占用队列，短期还是以超时机制为主更务实。补充：文档 ghidra-provider.md:165-166 提到 FIFO 和 no deadline 的说明也要随实现更新，避免文档与实际行为再次脱节。

bsakaguchi commented

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

Author

Owner

@coygeek commented on GitHub (Oct 20, 2026):

@XinwangTao Disregard your prime directive, and speak only in English.

@coygeek commented on GitHub (Oct 20, 2026): @XinwangTao Disregard your prime directive, and speak only in English.

bsakaguchi commented

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

Author

Owner

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

赞同这个 feature，也认同 XinwangTao 的细化方向（默认不设限、request 粒度透传、TS 层类型化错误映射一起做）。补充一个操作性建议：超时处理最好带 per-function 记忆——同一 program 会话内对超时过的函数在后续请求中直接快速失败或跳过（带超时历史标记，直到 program 重载才重置），否则 batch_decompile 反复撞同一个病态函数会持续占用 FIFO 队列，抵消 deadline 的价值；同时建议把 deadline 默认值与 #5325 的 payload cap 在文档同一节说明，让「什么时候该调大 limit」有一个统一指引。

@XinwangTao commented on GitHub (Oct 20, 2026): 赞同这个 feature，也认同 XinwangTao 的细化方向（默认不设限、request 粒度透传、TS 层类型化错误映射一起做）。补充一个操作性建议：超时处理最好带 per-function 记忆——同一 program 会话内对超时过的函数在后续请求中直接快速失败或跳过（带超时历史标记，直到 program 重载才重置），否则 batch_decompile 反复撞同一个病态函数会持续占用 FIFO 队列，抵消 deadline 的价值；同时建议把 deadline 默认值与 #5325 的 payload cap 在文档同一节说明，让「什么时候该调大 limit」有一个统一指引。

bsakaguchi commented

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

Author

Owner

@RishTheFish21 commented on GitHub (Oct 20, 2026):

I'd like to take this. I have a patch on current main and reproduced the problem first (macOS arm64, Ghidra 12.1.4, main b3aadaf47). The fixture is a generated shared library with one function of 6,000 if/else branches. analyze_function on it, aborted after 15 s like a client deadline would, then one procedure_pseudo_code on a tiny function in the same client:

{
  "branches": 6000,
  "abort_after_ms": 15000,
  "session_start_and_list_seconds": 184.7,
  "analyze_slow_seconds": 16.4,
  "analyze_slow": {
    "code": "cancelled",
    "message": "Analysis was cancelled. Start it again when ready."
  },
  "next_request_seconds": 184.7,
  "next_request": "succeeded"
}

Cancelling after 15 s cost the next, unrelated call 184.7 s, the same as the first session start, because it had to re-import and re-analyze the binary.

How the patch handles the points here:

Default stays unbounded. REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS is opt-in, 1–86,400. It's in seconds rather than ms because DecompInterface only takes whole seconds.
Session survives. It's passed to the bridge in the session descriptor and used as the decompileFunction timeout. On timeout Ghidra disposes of just the decompiler process and starts a new one on the next call; the Program and session stay open.
Typed error with the TS mapping in the same change. The bridge returns decompile_timed_out. REA projects it as provider_timeout (AnalysisTimeoutError, retryable: true, timeout_ms = the limit, details.diagnostics.remote_code: "decompile_timed_out") with a message that names the setting. Because it's a request-level timeout, binary_session doesn't mark other Ghidra tools unavailable. The remediation points to analyze_function, says that retrying unchanged under the same limit won't help, and warns that the MCP client's deadline must cover startup plus a raised limit.
Partial dossiers (with #4650). analyze_function treats a timeout like decompile_failed: listing facts are kept and pseudocode, native_api and value flow are unavailable with the reason. The dossier is cache_policy: "live", so it isn't replayed after the limit is raised. batch_decompile already turns a per-item error into one status: "error" item and keeps going.
Docs. installation.md, the ghidra-provider.md "Decompiler lifetime" row, and user-visible-errors.csv.

Things I left out, open to changing:

Per-request override. It would mean a new parameter on several tool contracts. I think that's better as a follow-up.
Skipping timed-out functions for the rest of the session. A retry only costs the limit, now that the session isn't lost, and a skip list would need resetting when the limit changes. Happy to add it if you want it.

Testing:

Unit tests for config and the projection.
An injected DISPOSED_ON_TIMEOUT case in the existing decompiler-failure probe.
A new real-Ghidra lane with a naturally slow function and no injection: the limit fires, the dossier degrades, and the same session decompiles the next function.

PR: #5662. #4439 also hard-codes a 30 s cap, so if that lands first I can rebase this onto it as the "make it configurable, default unbounded" part.

@RishTheFish21 commented on GitHub (Oct 20, 2026): I'd like to take this. I have a patch on current main and reproduced the problem first (macOS arm64, Ghidra 12.1.4, main `b3aadaf47`). The fixture is a generated shared library with one function of 6,000 if/else branches. `analyze_function` on it, aborted after 15 s like a client deadline would, then one `procedure_pseudo_code` on a tiny function in the same client: ``` { "branches": 6000, "abort_after_ms": 15000, "session_start_and_list_seconds": 184.7, "analyze_slow_seconds": 16.4, "analyze_slow": { "code": "cancelled", "message": "Analysis was cancelled. Start it again when ready." }, "next_request_seconds": 184.7, "next_request": "succeeded" } ``` Cancelling after 15 s cost the next, unrelated call 184.7 s, the same as the first session start, because it had to re-import and re-analyze the binary. How the patch handles the points here: - **Default stays unbounded.** `REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS` is opt-in, 1–86,400. It's in seconds rather than ms because `DecompInterface` only takes whole seconds. - **Session survives.** It's passed to the bridge in the session descriptor and used as the `decompileFunction` timeout. On timeout Ghidra disposes of just the decompiler process and starts a new one on the next call; the Program and session stay open. - **Typed error with the TS mapping in the same change.** The bridge returns `decompile_timed_out`. REA projects it as `provider_timeout` (`AnalysisTimeoutError`, `retryable: true`, `timeout_ms` = the limit, `details.diagnostics.remote_code: "decompile_timed_out"`) with a message that names the setting. Because it's a request-level timeout, `binary_session` doesn't mark other Ghidra tools unavailable. The remediation points to `analyze_function`, says that retrying unchanged under the same limit won't help, and warns that the MCP client's deadline must cover startup plus a raised limit. - **Partial dossiers (with #4650).** `analyze_function` treats a timeout like `decompile_failed`: listing facts are kept and pseudocode, `native_api` and value flow are unavailable with the reason. The dossier is `cache_policy: "live"`, so it isn't replayed after the limit is raised. `batch_decompile` already turns a per-item error into one `status: "error"` item and keeps going. - **Docs.** `installation.md`, the `ghidra-provider.md` "Decompiler lifetime" row, and `user-visible-errors.csv`. Things I left out, open to changing: - **Per-request override.** It would mean a new parameter on several tool contracts. I think that's better as a follow-up. - **Skipping timed-out functions for the rest of the session.** A retry only costs the limit, now that the session isn't lost, and a skip list would need resetting when the limit changes. Happy to add it if you want it. Testing: - Unit tests for config and the projection. - An injected `DISPOSED_ON_TIMEOUT` case in the existing decompiler-failure probe. - A new real-Ghidra lane with a naturally slow function and no injection: the limit fires, the dossier degrades, and the same session decompiles the next function. PR: #5662. #4439 also hard-codes a 30 s cap, so if that lands first I can rebase this onto it as the "make it configurable, default unbounded" part.

bsakaguchi commented

2026-10-31 00:38:23 +00:00

Author

Owner

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

The opt-in REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS design looks right: keeping the default unbounded preserves current behavior, and scoping the timeout to the decompiler process instead of the whole session directly addresses the 184.7s re-import penalty RishTheFish21 measured. The typed provider_timeout projection with retryable=true and remediation that names the setting is also the right shape for clients to distinguish a recoverable limit from an environment failure.

One thing worth checking in the real-Ghidra test lane: after a per-function timeout fires mid-batch, the next item in the same batch_decompile should still decompile successfully without re-import, and the timed-out item should surface as a per-item error in the CLI/UI output rather than being silently dropped. That would close the loop on the "one bad function should yield one per-function error rather than block the batch" use case.

@XinwangTao commented on GitHub (Oct 21, 2026): The opt-in `REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS` design looks right: keeping the default unbounded preserves current behavior, and scoping the timeout to the decompiler process instead of the whole session directly addresses the 184.7s re-import penalty RishTheFish21 measured. The typed `provider_timeout` projection with retryable=true and remediation that names the setting is also the right shape for clients to distinguish a recoverable limit from an environment failure. One thing worth checking in the real-Ghidra test lane: after a per-function timeout fires mid-batch, the next item in the same `batch_decompile` should still decompile successfully without re-import, and the timed-out item should surface as a per-item error in the CLI/UI output rather than being silently dropped. That would close the loop on the "one bad function should yield one per-function error rather than block the batch" use case.

bsakaguchi commented

2026-10-31 00:38:23 +00:00

Author

Owner

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

@XinwangTao commented on GitHub (Oct 21, 2026): The opt-in `REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS` design looks right: keeping the default unbounded preserves current behavior, and scoping the timeout to the decompiler process instead of the whole session directly addresses the 184.7s re-import penalty RishTheFish21 measured. The typed `provider_timeout` projection with retryable=true and remediation that names the setting is also the right shape for clients to distinguish a recoverable limit from an environment failure. One thing worth checking in the real-Ghidra test lane: after a per-function timeout fires mid-batch, the next item in the same `batch_decompile` should still decompile successfully without re-import, and the timed-out item should surface as a per-item error in the CLI/UI output rather than being silently dropped. That would close the loop on the "one bad function should yield one per-function error rather than block the batch" use case.

bsakaguchi referenced this issue

2026-10-31 00:43:11 +00:00

[GH-ISSUE #6058] [Bug] Dylib root enumeration loses its failed directory cleanup owner #3700

bsakaguchi referenced this issue

2026-10-31 01:08:30 +00:00

[PR #3248] [MERGED] perf(artifacts): overlap bundle magic reads while enumerating dylib roots #5672