morluto/rea

Fork 0

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

[GH-ISSUE #3983] [Bug] Concurrent Ghidra startup waits share the initiating request cancellation #1578

New issue

Closed

opened 2026-10-16 21:07:22 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-16 21:07:22 +00:00

Owner

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

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Native analysis or decompilation

Observed behavior

Two concurrent Ghidra-backed MCP reads can share the first request's cancellation outcome during cold startup. Cancelling the second request aborts its server-side signal but leaves its provider operation pending. Cancelling the initiating request instead makes an uncancelled second request fail with cancelled. Both outcomes were reproduced through registered MCP tools using a simulated installation and launcher with the production session, provider and Ghidra client.

Reproduction

From the baseline checkout, save the following reproduction test as src/ghidra/sharedStartupReproduction.test.ts. It opens the running Node executable as a native target through open_binary with provider_id: "ghidra", then sends two overlapping list_strings calls with empty arguments. The production client factory receives its ordinary target digest and committed profile. Only installation inspection and launcher completion are simulated.

import { existsSync } from "node:fs";
import { resolve } from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import { expect, it } from "vitest";
import { GhidraClient } from "./GhidraClient.js";
import { AnalysisCancelledError } from "../domain/analysisErrorCore.js";
import { err } from "../domain/result.js";
import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
import { composeBinarySession } from "../application/binary/BinarySessionComposition.js";
import { AnalysisProviderRegistry } from "../application/binary/AnalysisProviderRegistry.js";
import { parseConfig } from "../config/parseConfig.js";
import { GhidraProvider } from "./GhidraProvider.js";
import { ghidraProfileInstallationHost } from "./GhidraInstallation.fixture.js";
import { silentLogger } from "../logger.js";
import { createServer } from "../server/createServer.js";

it.each(["secondary_cancel", "owner_cancel"] as const)("public MCP shared startup: %s", async mode => {
  const entered = Promise.withResolvers<void>();
  const secondEntered = Promise.withResolvers<void>();
  const runtimeRoots: string[] = [];
  let calls = 0;
  let waiterSettled = false;
  let waiterSignal: AbortSignal | undefined;
  let startupSignal: AbortSignal | undefined;
  const config = parseConfig({ GHIDRA_INSTALL_DIR: resolve("fixture-ghidra-install"), REA_GHIDRA_STARTUP_TIMEOUT_MS: "5000" });
  if (!config.ok) throw config.error;
  const provider = new GhidraProvider(config.value, silentLogger, {}, ghidraProfileInstallationHost, options => {
    const raw = new GhidraClient({ ...options, platform: process.platform, launcher: {
      async launch(launchSession, launchOptions) {
        runtimeRoots.push(launchSession.runtimeRoot);
        entered.resolve();
        const signal = launchOptions?.signal;
        startupSignal = signal;
        await new Promise<void>(resolve => {
          if (signal?.aborted) resolve();
          else signal?.addEventListener("abort", () => resolve(), { once: true });
        });
        return err(new AnalysisCancelledError("open_binary"));
      },
    }});
    return {
      start: raw.start.bind(raw), close: raw.close.bind(raw),
      callTool: (...args) => {
        const pending = raw.callTool(...args);
        if (++calls === 2) {
          waiterSignal = args[2]?.signal;
          secondEntered.resolve();
          return pending.finally(() => { waiterSettled = true; });
        }
        return pending;
      },
    };
  });
  const session = composeBinarySession(new AnalysisProviderRegistry([provider]));
  const server = createServer({ kind: "session", session }, { logger: silentLogger });
  const mcp = new Client({ name: "startup-cancellation-reproduction", version: "1.0.0" });
  const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
  const owner = new AbortController();
  const waiter = new AbortController();
  let first: Promise<unknown> | undefined;
  let second: ReturnType<Client["callTool"]> | undefined;
  try {
    await server.connect(serverTransport);
    await mcp.connect(clientTransport);
    const opened = await mcp.callTool({ name: "open_binary", arguments: { path: process.execPath, provider_id: "ghidra" } });
    expect(opened.isError).not.toBe(true);
    first = mcp.callTool({ name: "list_strings", arguments: {} }, { signal: owner.signal }).catch(error => ({ clientAbort: error.message }));
    await entered.promise;
    second = mcp.callTool({ name: "list_strings", arguments: {} }, { signal: waiter.signal });
    void second.catch(() => {});
    await secondEntered.promise;
    if (mode === "secondary_cancel") {
      waiter.abort();
      await delay(100);
      console.log(JSON.stringify({case: "mcp_secondary_cancel", server_signal_aborted: waiterSignal?.aborted,
        provider_waiter_settled_after_100ms: waiterSettled, startup_aborted: startupSignal?.aborted}));
      expect(waiterSignal?.aborted).toBe(true);
      expect(waiterSettled).toBe(false);
      expect(startupSignal?.aborted).toBe(false);
      return;
    }
    owner.abort();
    const result = await second;
    const text = result.content.find(item => item.type === "text");
    if (!text || text.type !== "text") throw new Error("Missing error text");
    const error = JSON.parse(text.text).error;
    console.log(JSON.stringify({ case: "mcp_uncancelled_waiter", signal_aborted: waiter.signal.aborted, isError: result.isError, code: error.code, category: error.category }));
    expect(waiter.signal.aborted).toBe(false);
    expect(result.isError).toBe(true);
    expect(error.code).toBe("cancelled");
  } finally {
    owner.abort();
    await Promise.allSettled([first, second]);
    await Promise.allSettled([mcp.close(), server.close()]);
    await session.close();
    for (const root of runtimeRoots) expect(existsSync(root)).toBe(false);
  }
});

Run npx vitest run src/ghidra/sharedStartupReproduction.test.ts --maxWorkers=1 with the repository's pinned toolchain and installed dependencies. The two cases assert the defective observations, so passing establishes reproduction rather than a fix. Use disposable HOME and TMPDIR directories for the run. The local reproduction ran equivalent tests in isolation and deleted the temporary checkout test afterward.
In secondary_cancel, the second SDK call rejects after cancellation, but the server-side provider operation remains pending. The test confirms receipt of the abort on that operation's signal before checking settlement. The first caller stays interested and the launcher gate stays closed.
In owner_cancel, the second request remains uncancelled. It receives an MCP error whose code and category are both cancelled when the first request aborts. The test closes the client, server and session in both cases and checks removal of every runtime root captured by the simulated launcher.

Expected behavior

Each request's cancellation must apply while that request waits for cold startup. A cancelled secondary request should release its server-side provider wait without depending on the initiating request's signal. An uncancelled concurrent request should retain its own cancellation identity. If another request stopping the provider prevents completion, the surviving request needs a truthful provider outcome or recovery rather than being described as caller-cancelled. The contract does not require this report's particular observation interval or mandate a startup implementation.

Impact and scope

Overlapping cold reads retain cancelled server work or fail an uncancelled read. The reproduced defect is bounded to startup waiters; no data corruption, security boundary crossing, or product-wide workflow failure was observed.

Serializing initial reads appears to avoid concurrent startup waiters by source inspection. A successful serialized real-provider workflow was not executed, so its practical availability is unverified.

REA version

rea 6.5.0 at source commit 4692bb3330

Runtime and platform

Node 24.18.0; macOS arm64

Target and artifact context

Native Node executable used for admission only; simulated Ghidra installation and launcher; no engine or target execution.

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Expected source: docs/mcp-contracts.md:418 forwards the request cancellation signal; its "Ghidra first-query deadlines and recovery" section includes cold startup in an individual tool call's wait. docs/installation.md:861 describes operation completion on result, caller cancellation or provider shutdown. src/domain/analysisErrorCore.ts:138 defines AnalysisCancelledError as caller cancellation winning before completion.
Failure source: src/ghidra/GhidraClient.ts:194 selects the existing startup promise; only the initiating signal receives the listener at line 203. callTool at line 290 waits for startup before reaching the abort-aware request queue. The executed reproduction above observes both outcomes through registered MCP tools.

The public call path was also inspected. src/contracts/officialToolContracts.ts:126 admits empty list_strings arguments; src/server/registerOfficialTools.ts:115 forwards the MCP request signal into analysis. src/application/binary/BinarySession.ts:600 permits concurrent calls and awaits the active provider operation. src/ghidra/GhidraProviderClient.ts:827 forwards the individual signal to the production Ghidra client and projects the returned cancellation into the caller-cancellation error. The executed MCP tests passed admission and reached both overlapping provider operations.

The retry for an already-aborted startup does not protect a waiter that entered before the first request aborted. The separate warm-session FIFO cancellation promise applies after queue entry; these observations concern startup before queue admission. Runtime-root cleanup completed in the probes. Real Ghidra behavior remains outside the executed scope.

Both public MCP cases reproduced in the final focused run. The secondary request's server-side signal was aborted, but its provider promise stayed unsettled during the 100 ms observation while the startup signal remained live. The uncancelled second request returned isError: true with error.code: "cancelled" and error.category: "cancelled" after only the initiating request aborted. Direct Ghidra-client controls independently reproduced the same ownership behavior. The final focused invocation passed all four observation tests.

{"case":"mcp_secondary_cancel","server_signal_aborted":true,"provider_waiter_settled_after_100ms":false,"startup_aborted":false}
{"case":"mcp_uncancelled_waiter","signal_aborted":false,"isError":true,"code":"cancelled","category":"cancelled"}

The cancelling client's SDK promise rejects promptly. The retained work is the server's provider operation. The 100 ms check establishes that it cannot settle through its own cancellation while startup stays gated; it is not a promised latency threshold. No real-engine import duration, stdio cancellation trial, or maximum retained-wait duration was measured.

Typed outcome or recovery expected

Repeat both public MCP cases with one cold startup held pending. Cancelling only the secondary request must settle its server-side provider operation as cancelled without requiring the first request to abort or the launcher gate to open. Cancelling only the initiating request must not give the uncancelled request a caller-cancelled result merely because it shared startup. Verify truthful recovery or provider failure for that request, one owned startup at a time, runtime-root cleanup, and existing warm-session queued and active cancellation behavior. Keep the server-side settlement observation so immediate client SDK rejection cannot hide retained provider work.

Analysis or suspected cause

The source references above explain the observed behavior. Runtime claims are limited to the explicitly described simulation boundaries.

Additional context

Runtime claims above describe the supplied local reproduction at the baseline commit. Real-provider frequency remains unmeasured.

The relevant failure code is unchanged on current origin/main fc712b3750.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3983 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Native analysis or decompilation ### Observed behavior Two concurrent Ghidra-backed MCP reads can share the first request's cancellation outcome during cold startup. Cancelling the second request aborts its server-side signal but leaves its provider operation pending. Cancelling the initiating request instead makes an uncancelled second request fail with `cancelled`. Both outcomes were reproduced through registered MCP tools using a simulated installation and launcher with the production session, provider and Ghidra client. ### Reproduction 1. From the baseline checkout, save the following reproduction test as `src/ghidra/sharedStartupReproduction.test.ts`. It opens the running Node executable as a native target through `open_binary` with `provider_id: "ghidra"`, then sends two overlapping `list_strings` calls with empty arguments. The production client factory receives its ordinary target digest and committed profile. Only installation inspection and launcher completion are simulated. ```ts import { existsSync } from "node:fs"; import { resolve } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { expect, it } from "vitest"; import { GhidraClient } from "./GhidraClient.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { err } from "../domain/result.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { composeBinarySession } from "../application/binary/BinarySessionComposition.js"; import { AnalysisProviderRegistry } from "../application/binary/AnalysisProviderRegistry.js"; import { parseConfig } from "../config/parseConfig.js"; import { GhidraProvider } from "./GhidraProvider.js"; import { ghidraProfileInstallationHost } from "./GhidraInstallation.fixture.js"; import { silentLogger } from "../logger.js"; import { createServer } from "../server/createServer.js"; it.each(["secondary_cancel", "owner_cancel"] as const)("public MCP shared startup: %s", async mode => { const entered = Promise.withResolvers<void>(); const secondEntered = Promise.withResolvers<void>(); const runtimeRoots: string[] = []; let calls = 0; let waiterSettled = false; let waiterSignal: AbortSignal | undefined; let startupSignal: AbortSignal | undefined; const config = parseConfig({ GHIDRA_INSTALL_DIR: resolve("fixture-ghidra-install"), REA_GHIDRA_STARTUP_TIMEOUT_MS: "5000" }); if (!config.ok) throw config.error; const provider = new GhidraProvider(config.value, silentLogger, {}, ghidraProfileInstallationHost, options => { const raw = new GhidraClient({ ...options, platform: process.platform, launcher: { async launch(launchSession, launchOptions) { runtimeRoots.push(launchSession.runtimeRoot); entered.resolve(); const signal = launchOptions?.signal; startupSignal = signal; await new Promise<void>(resolve => { if (signal?.aborted) resolve(); else signal?.addEventListener("abort", () => resolve(), { once: true }); }); return err(new AnalysisCancelledError("open_binary")); }, }}); return { start: raw.start.bind(raw), close: raw.close.bind(raw), callTool: (...args) => { const pending = raw.callTool(...args); if (++calls === 2) { waiterSignal = args[2]?.signal; secondEntered.resolve(); return pending.finally(() => { waiterSettled = true; }); } return pending; }, }; }); const session = composeBinarySession(new AnalysisProviderRegistry([provider])); const server = createServer({ kind: "session", session }, { logger: silentLogger }); const mcp = new Client({ name: "startup-cancellation-reproduction", version: "1.0.0" }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); const owner = new AbortController(); const waiter = new AbortController(); let first: Promise<unknown> | undefined; let second: ReturnType<Client["callTool"]> | undefined; try { await server.connect(serverTransport); await mcp.connect(clientTransport); const opened = await mcp.callTool({ name: "open_binary", arguments: { path: process.execPath, provider_id: "ghidra" } }); expect(opened.isError).not.toBe(true); first = mcp.callTool({ name: "list_strings", arguments: {} }, { signal: owner.signal }).catch(error => ({ clientAbort: error.message })); await entered.promise; second = mcp.callTool({ name: "list_strings", arguments: {} }, { signal: waiter.signal }); void second.catch(() => {}); await secondEntered.promise; if (mode === "secondary_cancel") { waiter.abort(); await delay(100); console.log(JSON.stringify({case: "mcp_secondary_cancel", server_signal_aborted: waiterSignal?.aborted, provider_waiter_settled_after_100ms: waiterSettled, startup_aborted: startupSignal?.aborted})); expect(waiterSignal?.aborted).toBe(true); expect(waiterSettled).toBe(false); expect(startupSignal?.aborted).toBe(false); return; } owner.abort(); const result = await second; const text = result.content.find(item => item.type === "text"); if (!text || text.type !== "text") throw new Error("Missing error text"); const error = JSON.parse(text.text).error; console.log(JSON.stringify({ case: "mcp_uncancelled_waiter", signal_aborted: waiter.signal.aborted, isError: result.isError, code: error.code, category: error.category })); expect(waiter.signal.aborted).toBe(false); expect(result.isError).toBe(true); expect(error.code).toBe("cancelled"); } finally { owner.abort(); await Promise.allSettled([first, second]); await Promise.allSettled([mcp.close(), server.close()]); await session.close(); for (const root of runtimeRoots) expect(existsSync(root)).toBe(false); } }); ``` 2. Run `npx vitest run src/ghidra/sharedStartupReproduction.test.ts --maxWorkers=1` with the repository's pinned toolchain and installed dependencies. The two cases assert the defective observations, so passing establishes reproduction rather than a fix. Use disposable HOME and TMPDIR directories for the run. The local reproduction ran equivalent tests in isolation and deleted the temporary checkout test afterward. 3. In `secondary_cancel`, the second SDK call rejects after cancellation, but the server-side provider operation remains pending. The test confirms receipt of the abort on that operation's signal before checking settlement. The first caller stays interested and the launcher gate stays closed. 4. In `owner_cancel`, the second request remains uncancelled. It receives an MCP error whose code and category are both `cancelled` when the first request aborts. The test closes the client, server and session in both cases and checks removal of every runtime root captured by the simulated launcher. ### Expected behavior Each request's cancellation must apply while that request waits for cold startup. A cancelled secondary request should release its server-side provider wait without depending on the initiating request's signal. An uncancelled concurrent request should retain its own cancellation identity. If another request stopping the provider prevents completion, the surviving request needs a truthful provider outcome or recovery rather than being described as caller-cancelled. The contract does not require this report's particular observation interval or mandate a startup implementation. ### Impact and scope Overlapping cold reads retain cancelled server work or fail an uncancelled read. The reproduced defect is bounded to startup waiters; no data corruption, security boundary crossing, or product-wide workflow failure was observed. Serializing initial reads appears to avoid concurrent startup waiters by source inspection. A successful serialized real-provider workflow was not executed, so its practical availability is unverified. ### REA version rea 6.5.0 at source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860 ### Runtime and platform Node 24.18.0; macOS arm64 ### Target and artifact context Native Node executable used for admission only; simulated Ghidra installation and launcher; no engine or target execution. ### Interface MCP ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence - Expected source: `docs/mcp-contracts.md:418` forwards the request cancellation signal; its "Ghidra first-query deadlines and recovery" section includes cold startup in an individual tool call's wait. `docs/installation.md:861` describes operation completion on result, caller cancellation or provider shutdown. `src/domain/analysisErrorCore.ts:138` defines AnalysisCancelledError as caller cancellation winning before completion. - Failure source: `src/ghidra/GhidraClient.ts:194` selects the existing startup promise; only the initiating signal receives the listener at line 203. `callTool` at line 290 waits for startup before reaching the abort-aware request queue. The executed reproduction above observes both outcomes through registered MCP tools. The public call path was also inspected. `src/contracts/officialToolContracts.ts:126` admits empty list_strings arguments; `src/server/registerOfficialTools.ts:115` forwards the MCP request signal into analysis. `src/application/binary/BinarySession.ts:600` permits concurrent calls and awaits the active provider operation. `src/ghidra/GhidraProviderClient.ts:827` forwards the individual signal to the production Ghidra client and projects the returned cancellation into the caller-cancellation error. The executed MCP tests passed admission and reached both overlapping provider operations. The retry for an already-aborted startup does not protect a waiter that entered before the first request aborted. The separate warm-session FIFO cancellation promise applies after queue entry; these observations concern startup before queue admission. Runtime-root cleanup completed in the probes. Real Ghidra behavior remains outside the executed scope. Both public MCP cases reproduced in the final focused run. The secondary request's server-side signal was aborted, but its provider promise stayed unsettled during the 100 ms observation while the startup signal remained live. The uncancelled second request returned `isError: true` with `error.code: "cancelled"` and `error.category: "cancelled"` after only the initiating request aborted. Direct Ghidra-client controls independently reproduced the same ownership behavior. The final focused invocation passed all four observation tests. ```json {"case":"mcp_secondary_cancel","server_signal_aborted":true,"provider_waiter_settled_after_100ms":false,"startup_aborted":false} {"case":"mcp_uncancelled_waiter","signal_aborted":false,"isError":true,"code":"cancelled","category":"cancelled"} ``` The cancelling client's SDK promise rejects promptly. The retained work is the server's provider operation. The 100 ms check establishes that it cannot settle through its own cancellation while startup stays gated; it is not a promised latency threshold. No real-engine import duration, stdio cancellation trial, or maximum retained-wait duration was measured. ### Typed outcome or recovery expected Repeat both public MCP cases with one cold startup held pending. Cancelling only the secondary request must settle its server-side provider operation as cancelled without requiring the first request to abort or the launcher gate to open. Cancelling only the initiating request must not give the uncancelled request a caller-cancelled result merely because it shared startup. Verify truthful recovery or provider failure for that request, one owned startup at a time, runtime-root cleanup, and existing warm-session queued and active cancellation behavior. Keep the server-side settlement observation so immediate client SDK rejection cannot hide retained provider work. ### Analysis or suspected cause The source references above explain the observed behavior. Runtime claims are limited to the explicitly described simulation boundaries. ### Additional context Runtime claims above describe the supplied local reproduction at the baseline commit. Real-provider frequency remains unmeasured. The relevant failure code is unchanged on current `origin/main` fc712b3750cbe16755c4f78c1f90fdf09fbaca31.

bsakaguchi closed this issue

2026-10-16 21:07:22 +00:00

bsakaguchi commented

2026-10-16 21:07:23 +00:00

Author

Owner

@yankikucuk commented on GitHub (Oct 15, 2026):

Working on this. Reproduced with your MCP test on current main (both cases). Startup waits are now joined per caller: a follower's own cancellation settles only its wait while the initiator keeps the startup; the startup is abandoned only when no caller remains interested, and the last caller to withdraw still gets the startup's own cancelled result with cleanup and diagnostics; an uncancelled follower whose startup is stopped by a close or by other callers withdrawing receives a start failure rather than a cancellation. Covered by a new MCP boundary test with the production provider, client and session routing; the Ghidra suite set shows the same environment-bound failures as main. Simulated launcher only. Draft PR follows shortly.

@yankikucuk commented on GitHub (Oct 15, 2026): Working on this. Reproduced with your MCP test on current main (both cases). Startup waits are now joined per caller: a follower's own cancellation settles only its wait while the initiator keeps the startup; the startup is abandoned only when no caller remains interested, and the last caller to withdraw still gets the startup's own cancelled result with cleanup and diagnostics; an uncancelled follower whose startup is stopped by a close or by other callers withdrawing receives a start failure rather than a cancellation. Covered by a new MCP boundary test with the production provider, client and session routing; the Ghidra suite set shows the same environment-bound failures as main. Simulated launcher only. Draft PR follows shortly.

bsakaguchi commented

2026-10-16 21:07:23 +00:00

Author

Owner

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

Fixed on main in ca7ee018b. Concurrent startup callers now own independent cancellation waits over one group-owned producer. Final departure joins cleanup; a departing caller no longer cancels or blocks its live peer.

The affected selection passed 315 tests, including public MCP peer cancellation, warm reuse and cleanup controls. Bridges were controlled child processes, not a real Ghidra JVM or Hopper GUI.

Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

@morluto commented on GitHub (Oct 16, 2026): Fixed on `main` in [ca7ee018b](https://github.com/morluto/rea/commit/ca7ee018b25db6074ab1cb31398af2e82d5a7c29). Concurrent startup callers now own independent cancellation waits over one group-owned producer. Final departure joins cleanup; a departing caller no longer cancels or blocks its live peer. The affected selection passed 315 tests, including public MCP peer cancellation, warm reuse and cleanup controls. Bridges were controlled child processes, not a real Ghidra JVM or Hopper GUI. Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

bsakaguchi referenced this issue

2026-10-17 21:12:35 +00:00

[PR #1579] [CLOSED] fix(browser): seed storage once per scenario storage area #2986

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

No description provided.

Rows
Columns