morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[GH-ISSUE #4860] [Bug] IDA regex searches block cancellation on the service thread #3021

New issue

Closed

opened 2026-10-31 00:34:59 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

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

Owner

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

Area

Native analysis or decompilation; cleanup, cancellation, or lifecycle.

Observed behavior

IDA search_procedures and search_strings perform caller-supplied regex matching synchronously on REA's service thread. A CPU-bound pattern prevents a scheduled AbortController cancellation from running; the operation returns success instead of cancellation.

Observed at the production IdaSessionClient.execute boundary with the existing RecordingIdaMcp fixture. This is not a real IDA engine or public CLI/MCP reproduction.

Reproduction

At revision d5be402fb3dbfc56ac9854c27b5538df20cc2437, use the existing attached IDA fixture in src/ida/IdaSessionClient.test.ts.
Return one function name (or string value) equal to "a".repeat(26) + "!" from list_functions (or list_strings). Keep the fixture's normal metadata and identity responses.
Initialize the session with execute("health", {}). Schedule controller.abort() after 50 ms.
Call execute(operation, { pattern: "^(a+)+$", mode: "regex" }, { signal: controller.signal }).
Both added cancellation regression cases fail on the base: result.ok is true, expected false. The procedure case took 3077 ms and the string case 594 ms in the isolated test run. These are total test durations, not isolated matching benchmarks.

Expected behavior

Keep the service thread available for cancellation. Matching should have a bounded lifetime and return a typed cancellation or resource-limit error, with the session still usable afterward. The existing src/process/RegexMatcher.ts already provides this behavior for other providers, including a five-second matching deadline and awaited worker termination.

Impact and scope

Executed reproduction covers both attached legacy IDA search operations through the production session client. The common runner also serves modern/native profiles (source-derived scope); those profiles and actual IDA engines were not exercised. Longer pathological inputs can require much more matching work; no unbounded-duration runtime measurement is claimed. This is a caller-controlled local resource/cancellation bug.

REA version

Source revision d5be402fb3dbfc56ac9854c27b5538df20cc2437, package version 6.5.0.

Runtime and platform

Windows x64, Node.js 24.14.1, npm 11.11.0. No IDA engine installed for this check.

Target and artifact context

Existing synthetic ELF fixture; CPU-bound function-name/string search.

Interface

Other: production provider adapter boundary, not public CLI/MCP.

Earliest failure stage

Cleanup, cancellation, or lifecycle: local inventory matching.

Relevant output or evidence

Executed baseline command after adding only the two cancellation tests:

node node_modules/vitest/vitest.mjs run src/ida/IdaSessionClient.test.ts -t "cancels CPU-bound" --reporter=verbose
2 failed | 61 skipped
AssertionError: expected true to be false

Source-derived cause: IdaOperationRunner.run obtains analysisSearchPredicate(query) and filters inventories synchronously; regex mode in analysisSearchPredicate calls RegExp.test on the service thread. The provider signal cannot interrupt this matching.

Typed outcome or recovery expected

AnalysisCancelledError on caller cancellation; AnalysisResourceConstraintError when the supervised matching deadline expires. Subsequent searches should work in the same session.

Proposed fix

Route regex inventories through the existing matchRegexValues worker, pass the caller signal, preserve literal matching and source order, and advertise the shared matching limitation. A companion draft PR will include the failing baseline tests and recovery/deadline coverage.

Originally created by @jgt87 on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4860 ### Area Native analysis or decompilation; cleanup, cancellation, or lifecycle. ### Observed behavior IDA `search_procedures` and `search_strings` perform caller-supplied regex matching synchronously on REA's service thread. A CPU-bound pattern prevents a scheduled AbortController cancellation from running; the operation returns success instead of cancellation. Observed at the production `IdaSessionClient.execute` boundary with the existing `RecordingIdaMcp` fixture. This is not a real IDA engine or public CLI/MCP reproduction. ### Reproduction 1. At revision `d5be402fb3dbfc56ac9854c27b5538df20cc2437`, use the existing attached IDA fixture in `src/ida/IdaSessionClient.test.ts`. 2. Return one function name (or string value) equal to `"a".repeat(26) + "!"` from `list_functions` (or `list_strings`). Keep the fixture's normal metadata and identity responses. 3. Initialize the session with `execute("health", {})`. Schedule `controller.abort()` after 50 ms. 4. Call `execute(operation, { pattern: "^(a+)+$", mode: "regex" }, { signal: controller.signal })`. 5. Both added cancellation regression cases fail on the base: `result.ok` is `true`, expected `false`. The procedure case took 3077 ms and the string case 594 ms in the isolated test run. These are total test durations, not isolated matching benchmarks. ### Expected behavior Keep the service thread available for cancellation. Matching should have a bounded lifetime and return a typed cancellation or resource-limit error, with the session still usable afterward. The existing `src/process/RegexMatcher.ts` already provides this behavior for other providers, including a five-second matching deadline and awaited worker termination. ### Impact and scope Executed reproduction covers both attached legacy IDA search operations through the production session client. The common runner also serves modern/native profiles (source-derived scope); those profiles and actual IDA engines were not exercised. Longer pathological inputs can require much more matching work; no unbounded-duration runtime measurement is claimed. This is a caller-controlled local resource/cancellation bug. ### REA version Source revision `d5be402fb3dbfc56ac9854c27b5538df20cc2437`, package version 6.5.0. ### Runtime and platform Windows x64, Node.js 24.14.1, npm 11.11.0. No IDA engine installed for this check. ### Target and artifact context Existing synthetic ELF fixture; CPU-bound function-name/string search. ### Interface Other: production provider adapter boundary, not public CLI/MCP. ### Earliest failure stage Cleanup, cancellation, or lifecycle: local inventory matching. ### Relevant output or evidence Executed baseline command after adding only the two cancellation tests: ```text node node_modules/vitest/vitest.mjs run src/ida/IdaSessionClient.test.ts -t "cancels CPU-bound" --reporter=verbose 2 failed | 61 skipped AssertionError: expected true to be false ``` Source-derived cause: `IdaOperationRunner.run` obtains `analysisSearchPredicate(query)` and filters inventories synchronously; regex mode in `analysisSearchPredicate` calls `RegExp.test` on the service thread. The provider signal cannot interrupt this matching. ### Typed outcome or recovery expected `AnalysisCancelledError` on caller cancellation; `AnalysisResourceConstraintError` when the supervised matching deadline expires. Subsequent searches should work in the same session. ### Proposed fix Route regex inventories through the existing `matchRegexValues` worker, pass the caller signal, preserve literal matching and source order, and advertise the shared matching limitation. A companion draft PR will include the failing baseline tests and recovery/deadline coverage.

bsakaguchi closed this issue

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

bsakaguchi commented

2026-10-31 00:35:00 +00:00

Author

Owner

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

Confirmed root cause: analysisSearchPredicate runs caller-supplied RegExp.test synchronously on the service thread, so a CPU-bound pattern (like ^(a+)+$ against long strings) starves the event loop and AbortController never gets a chance to run. Routing regex inventory through the existing matchRegexValues worker with the signal passed through is the right fix — it reuses the proven bounded-lifetime behavior (5s deadline + worker termination) instead of introducing a second cancellation mechanism.

Two suggestions for the fix PR: (1) assert the typed outcomes separately — AnalysisCancelledError on abort vs AnalysisResourceConstraintError on deadline, since the two paths have different retry semantics; (2) after a cancelled search, add a follow-up "session still usable" test that runs a normal literal search in the same session, since the issue explicitly promises that as part of expected behavior.

@XinwangTao commented on GitHub (Oct 18, 2026): Confirmed root cause: `analysisSearchPredicate` runs caller-supplied `RegExp.test` synchronously on the service thread, so a CPU-bound pattern (like `^(a+)+$` against long strings) starves the event loop and AbortController never gets a chance to run. Routing regex inventory through the existing `matchRegexValues` worker with the signal passed through is the right fix — it reuses the proven bounded-lifetime behavior (5s deadline + worker termination) instead of introducing a second cancellation mechanism. Two suggestions for the fix PR: (1) assert the typed outcomes separately — `AnalysisCancelledError` on abort vs `AnalysisResourceConstraintError` on deadline, since the two paths have different retry semantics; (2) after a cancelled search, add a follow-up "session still usable" test that runs a normal literal search in the same session, since the issue explicitly promises that as part of expected behavior.

bsakaguchi referenced this issue

2026-10-31 01:07:36 +00:00

[PR #3021] [MERGED] docs(readme): restore Greek guide and official-source links #5562

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#3021

No description provided.

Rows
Columns