[GH-ISSUE #4457] [Bug] PE resource provider closes while admitted parsing is still active

bsakaguchi commented

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

Owner

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

Before submitting

Searched open/closed issues and PRs for PeResourcesProvider, PE resources close, and PE lifetime. Related resource-mapping reports #4373 and #4455 concern different behavior.
No credentials, private targets, or ambient environment are included.

Area

Native analysis or decompilation

Observed behavior

PeResourcesProvider.close() resolves while an already accepted inspect() is still awaiting resource parsing. The file read is admitted to ArtifactResourceScope.run, but parsing runs after that admission ends. Close therefore drains no active operation at that stage.

Observed at the production provider boundary using a real temporary PE file and the real parser, with a deterministic gate immediately before parser entry. Both normal-completion and cancellation controls reported close completion before the gate was released. This establishes the lifetime gap; it does not measure its frequency in an ordinary CLI/MCP shutdown. No descriptor leak or incorrect resource bytes were observed.

Reproduction

Baseline provider source: main fe2a398dd. Executed with pinned Node 24.18.0 on macOS arm64 and locked dependencies. Save the following as tests/process-global/peResourcesLifetime.test.ts and run:

npm run test:focused -- tests/process-global/peResourcesLifetime.test.ts

Result before repair: 2 tests failed with close reported release while parsing was still admitted: expected true to be false. The gate is the only substituted behavior; stable file acquisition, parser result/cancellation, and cleanup scope are production code. The test releases the gate and joins the inspection/close in finally.

Complete controlled reproducer

import { writeFile } from "node:fs/promises";
import { join } from "node:path";
import { setImmediate } from "node:timers/promises";
import { beforeEach, expect, it, vi } from "vitest";

const parsing = vi.hoisted(() => {
  const gate = () => {
    let resolve!: () => void;
    const promise = new Promise<void>((complete) => {
      resolve = complete;
    });
    return { promise, resolve };
  };
  return { gate, entered: gate(), release: gate() };
});

vi.mock("../../src/native/pe/PeResourceParser.js", async (importOriginal) => {
  const original =
    await importOriginal<
      typeof import("../../src/native/pe/PeResourceParser.js")
    >();
  return {
    ...original,
    parsePeResources: async (
      ...arguments_: Parameters<typeof original.parsePeResources>
    ) => {
      parsing.entered.resolve();
      await parsing.release.promise;
      return original.parsePeResources(...arguments_);
    },
  };
});

import { inspectPeResourcesInputSchema } from "../../src/domain/native/peResources.js";
import { PeResourcesProvider } from "../../src/native/pe/PeResourcesProvider.js";
import { peResourceFixture } from "../../src/native/pe/PeResources.fixture.js";
import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";

beforeEach(() => {
  parsing.entered = parsing.gate();
  parsing.release = parsing.gate();
});

it.each(["completion", "cancellation"])(
  "waits for admitted PE parsing through %s before closing",
  async (outcome) => {
    const root = await createTestTempDirectory("rea-pe-parsing-lifetime-");
    const path = join(root, "image.exe");
    await writeFile(path, peResourceFixture().bytes);
    const provider = new PeResourcesProvider();
    const controller = new AbortController();
    const reason = new Error("cancelled during admitted parsing");
    const inspection = provider.inspect(
      inspectPeResourcesInputSchema.parse({ path }),
      { signal: controller.signal },
    );
    await parsing.entered.promise;
    let closed = false;
    const closing = provider.close().then(() => {
      closed = true;
    });
    try {
      if (outcome === "cancellation") controller.abort(reason);
      await setImmediate();
      expect(
        closed,
        "close reported release while parsing was still admitted",
      ).toBe(false);
      parsing.release.resolve();
      const result = await inspection;
      if (outcome === "cancellation")
        expect(result).toMatchObject({
          ok: false,
          error: { _tag: "AnalysisCancelledError", cause: reason },
        });
      else
        expect(result).toMatchObject({
          ok: true,
          value: { coverage: { status: "complete" } },
        });
      await closing;
      expect(closed).toBe(true);
      expect(
        (await provider.inspect(inspectPeResourcesInputSchema.parse({ path })))
          .ok,
      ).toBe(false);
    } finally {
      parsing.release.resolve();
      await inspection;
      await closing;
    }
  },
);

Expected behavior

Keep the complete admitted inspection, including asynchronous parsing, within the provider's operation lifetime. Close should await completion or cancellation before reporting release, and reject subsequent admission. This follows REA's lifecycle contract: admit the entire inspection once, including later work and finalization.

Impact and scope

Observed: premature provider-close completion in the deterministic boundary interleaving. Source-derived: ordinary parser traversal yields asynchronously, so parsing can remain active after stable acquisition. NOT_ENOUGH_INFO for a public CLI/MCP shutdown reproduction, native provider execution, real-world frequency, or leaked resources; none were demonstrated.

Relevant output or evidence

PeResourcesProvider.inspect places only readStableArtifact inside resources.run, then awaits parsePeResources outside it. ArtifactResourceScope.close waits only admitted active operations.

Typed outcome or recovery expected

Both controls should retain a pending close while parsing is gated; after release, the normal inspection completes and the cancelled inspection retains its typed cancellation and original cause. Close then resolves and new work remains refused. Move the existing admission around the whole inspection rather than introducing another owner or re-entering a draining scope.

Originally created by @morluto on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4457 ### Before submitting - [x] Searched open/closed issues and PRs for `PeResourcesProvider`, PE resources close, and PE lifetime. Related resource-mapping reports #4373 and #4455 concern different behavior. - [x] No credentials, private targets, or ambient environment are included. ### Area Native analysis or decompilation ### Observed behavior `PeResourcesProvider.close()` resolves while an already accepted `inspect()` is still awaiting resource parsing. The file read is admitted to `ArtifactResourceScope.run`, but parsing runs after that admission ends. Close therefore drains no active operation at that stage. Observed at the production provider boundary using a real temporary PE file and the real parser, with a deterministic gate immediately before parser entry. Both normal-completion and cancellation controls reported close completion before the gate was released. This establishes the lifetime gap; it does not measure its frequency in an ordinary CLI/MCP shutdown. No descriptor leak or incorrect resource bytes were observed. ### Reproduction Baseline provider source: main `fe2a398dd`. Executed with pinned Node 24.18.0 on macOS arm64 and locked dependencies. Save the following as `tests/process-global/peResourcesLifetime.test.ts` and run: ```sh npm run test:focused -- tests/process-global/peResourcesLifetime.test.ts ``` Result before repair: 2 tests failed with `close reported release while parsing was still admitted: expected true to be false`. The gate is the only substituted behavior; stable file acquisition, parser result/cancellation, and cleanup scope are production code. The test releases the gate and joins the inspection/close in `finally`. <details> <summary>Complete controlled reproducer</summary> ```typescript import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { setImmediate } from "node:timers/promises"; import { beforeEach, expect, it, vi } from "vitest"; const parsing = vi.hoisted(() => { const gate = () => { let resolve!: () => void; const promise = new Promise<void>((complete) => { resolve = complete; }); return { promise, resolve }; }; return { gate, entered: gate(), release: gate() }; }); vi.mock("../../src/native/pe/PeResourceParser.js", async (importOriginal) => { const original = await importOriginal< typeof import("../../src/native/pe/PeResourceParser.js") >(); return { ...original, parsePeResources: async ( ...arguments_: Parameters<typeof original.parsePeResources> ) => { parsing.entered.resolve(); await parsing.release.promise; return original.parsePeResources(...arguments_); }, }; }); import { inspectPeResourcesInputSchema } from "../../src/domain/native/peResources.js"; import { PeResourcesProvider } from "../../src/native/pe/PeResourcesProvider.js"; import { peResourceFixture } from "../../src/native/pe/PeResources.fixture.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; beforeEach(() => { parsing.entered = parsing.gate(); parsing.release = parsing.gate(); }); it.each(["completion", "cancellation"])( "waits for admitted PE parsing through %s before closing", async (outcome) => { const root = await createTestTempDirectory("rea-pe-parsing-lifetime-"); const path = join(root, "image.exe"); await writeFile(path, peResourceFixture().bytes); const provider = new PeResourcesProvider(); const controller = new AbortController(); const reason = new Error("cancelled during admitted parsing"); const inspection = provider.inspect( inspectPeResourcesInputSchema.parse({ path }), { signal: controller.signal }, ); await parsing.entered.promise; let closed = false; const closing = provider.close().then(() => { closed = true; }); try { if (outcome === "cancellation") controller.abort(reason); await setImmediate(); expect( closed, "close reported release while parsing was still admitted", ).toBe(false); parsing.release.resolve(); const result = await inspection; if (outcome === "cancellation") expect(result).toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError", cause: reason }, }); else expect(result).toMatchObject({ ok: true, value: { coverage: { status: "complete" } }, }); await closing; expect(closed).toBe(true); expect( (await provider.inspect(inspectPeResourcesInputSchema.parse({ path }))) .ok, ).toBe(false); } finally { parsing.release.resolve(); await inspection; await closing; } }, ); ``` </details> ### Expected behavior Keep the complete admitted inspection, including asynchronous parsing, within the provider's operation lifetime. Close should await completion or cancellation before reporting release, and reject subsequent admission. This follows [REA's lifecycle contract](https://github.com/morluto/rea/blob/fe2a398dd/docs/tool-design.md#own-the-lifecycle): admit the entire inspection once, including later work and finalization. ### Impact and scope Observed: premature provider-close completion in the deterministic boundary interleaving. Source-derived: ordinary parser traversal yields asynchronously, so parsing can remain active after stable acquisition. NOT_ENOUGH_INFO for a public CLI/MCP shutdown reproduction, native provider execution, real-world frequency, or leaked resources; none were demonstrated. ### Relevant output or evidence [PeResourcesProvider.inspect](https://github.com/morluto/rea/blob/fe2a398dd/src/native/pe/PeResourcesProvider.ts#L30-L55) places only `readStableArtifact` inside `resources.run`, then awaits `parsePeResources` outside it. [ArtifactResourceScope.close](https://github.com/morluto/rea/blob/fe2a398dd/src/artifacts/ArtifactResourceScope.ts) waits only admitted active operations. ### Typed outcome or recovery expected Both controls should retain a pending close while parsing is gated; after release, the normal inspection completes and the cancelled inspection retains its typed cancellation and original cause. Close then resolves and new work remains refused. Move the existing admission around the whole inspection rather than introducing another owner or re-entering a draining scope.

bsakaguchi added the

triage

bug

labels

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

bsakaguchi commented

2026-10-16 21:10:01 +00:00

Author

Owner

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

Root Cause & Reproduction

In src/native/pe/PeResourcesProvider.ts:

const artifact = await this.#resources.run(async () => {
  return await readStableArtifact(path, maxBytes, cancellation, this.#fs, this.#resources);
});
const resources = await parsePeResources(artifact.bytes, cancellation);

readStableArtifact was wrapped inside this.#resources.run(...), but the asynchronous parsePeResources(artifact.bytes, cancellation) executed outside the resource scope. When provider.close() was invoked while parsing was in flight, ArtifactResourceScope.close() observed 0 active tasks in #resources and resolved immediately, causing the provider to report complete release while background inspection was still active.

Verification & Regression Test

Added reproduction suite tests/process-global/peResourcesLifetime.test.ts verifying:

close() waits for normal-completion parsing before resolving.
close() waits for cancelled parsing before resolving, correctly propagating AnalysisCancelledError.
Subsequent inspections after close() are rejected.

Before the fix, both tests failed:

AssertionError: close reported release while parsing was still admitted: expected true to be false

With the fix, both tests pass cleanly, and all 28 existing PE tests in src/native/pe pass.

Fix

In src/native/pe/PeResourcesProvider.ts:

const resources = await this.#resources.run(async () => {
  const artifact = await readStableArtifact(
    path,
    maxBytes,
    cancellation,
    this.#fs,
    this.#resources,
  );
  return await parsePeResources(artifact.bytes, cancellation);
});

Ready Branch

Branch: qorvr:fix/pe-resources-provider-close-active-parsing (commit 753f0fc69)
Validated cleanly against upstream main (fe2a398dd) with npm run check:fast and verify:test-discovery.

(Standing by to open a PR as soon as our current 3 open PR limit allows, or maintainers may cherry-pick directly from the branch above.)

@qorvr commented on GitHub (Oct 16, 2026): ### Root Cause & Reproduction In `src/native/pe/PeResourcesProvider.ts`: ```typescript const artifact = await this.#resources.run(async () => { return await readStableArtifact(path, maxBytes, cancellation, this.#fs, this.#resources); }); const resources = await parsePeResources(artifact.bytes, cancellation); ``` `readStableArtifact` was wrapped inside `this.#resources.run(...)`, but the asynchronous `parsePeResources(artifact.bytes, cancellation)` executed outside the resource scope. When `provider.close()` was invoked while parsing was in flight, `ArtifactResourceScope.close()` observed 0 active tasks in `#resources` and resolved immediately, causing the provider to report complete release while background inspection was still active. ### Verification & Regression Test Added reproduction suite `tests/process-global/peResourcesLifetime.test.ts` verifying: 1. `close()` waits for normal-completion parsing before resolving. 2. `close()` waits for cancelled parsing before resolving, correctly propagating `AnalysisCancelledError`. 3. Subsequent inspections after `close()` are rejected. Before the fix, both tests failed: ``` AssertionError: close reported release while parsing was still admitted: expected true to be false ``` With the fix, both tests pass cleanly, and all 28 existing PE tests in `src/native/pe` pass. ### Fix In `src/native/pe/PeResourcesProvider.ts`: ```typescript const resources = await this.#resources.run(async () => { const artifact = await readStableArtifact( path, maxBytes, cancellation, this.#fs, this.#resources, ); return await parsePeResources(artifact.bytes, cancellation); }); ``` ### Ready Branch Branch: [`qorvr:fix/pe-resources-provider-close-active-parsing`](https://github.com/qorvr/rea/tree/fix/pe-resources-provider-close-active-parsing) (commit `753f0fc69`) Validated cleanly against upstream `main` (`fe2a398dd`) with `npm run check:fast` and `verify:test-discovery`. (Standing by to open a PR as soon as our current 3 open PR limit allows, or maintainers may cherry-pick directly from the branch above.)

bsakaguchi commented

2026-10-17 21:03:28 +00:00

Author

Owner

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

Cross-referencing with sibling PE provider issue #4455 ([Bug] PE resource mapping treats zero-extent sections as overlapping byte ranges). Both issues address defects in src/native/pe/PeResourcesProvider.ts regarding PE resource extraction, section bounds, and active parsing lifecycle.

Consolidated under Cluster U (PE Resource Parsing, Section Mapping & Lifecycle) in tracker #4330.

@matbrgz commented on GitHub (Oct 16, 2026): Cross-referencing with sibling PE provider issue #4455 (`[Bug] PE resource mapping treats zero-extent sections as overlapping byte ranges`). Both issues address defects in `src/native/pe/PeResourcesProvider.ts` regarding PE resource extraction, section bounds, and active parsing lifecycle. Consolidated under **Cluster U (PE Resource Parsing, Section Mapping & Lifecycle)** in tracker #4330.

bsakaguchi commented

2026-10-17 21:03:29 +00:00

Author

Owner

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026):

This appears fixed on current main (2092f90b7) by effb71f59 (the commit message references this issue).

Executed on main 2092f90b7 (macOS arm64, Node 24): npm run test:focused -- … including tests/process-global/peResourcesLifetime.test.ts (2 tests) — all passed (205/205 across the 13 files checked in this batch). Source check: PeResourcesProvider.inspect now runs parsePeResources inside this.#resources.run(...), so close() drains it.

I did not rerun the original fault injection from the report separately; the tests above are the regressions added with the fix. Leaving closure to a maintainer.

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026): This appears fixed on current main (2092f90b7) by effb71f59 (the commit message references this issue). Executed on main 2092f90b7 (macOS arm64, Node 24): `npm run test:focused -- …` including `tests/process-global/peResourcesLifetime.test.ts` (2 tests) — all passed (205/205 across the 13 files checked in this batch). Source check: `PeResourcesProvider.inspect` now runs `parsePeResources` inside `this.#resources.run(...)`, so `close()` drains it. I did not rerun the original fault injection from the report separately; the tests above are the regressions added with the fix. Leaving closure to a maintainer.

bsakaguchi closed this issue

2026-10-31 00:32:40 +00:00

bsakaguchi commented

2026-10-31 00:32:41 +00:00

Author

Owner

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

Reconciled against main94bc7ed3222ab6ca33fe68f58326eefb91689904. The repair and original gated regression were published in effb71f59c.

Current PeResourcesProvider.inspect admits stable acquisition and the awaited parser inside the same ArtifactResourceScope.run operation. close drains that entire operation and rejects later admission. The checked-in peResourcesLifetime controls use a real authored PE file and real parser, with only parser-entry scheduling gated. Both completion and cancellation controls passed; close stays pending until parsing settles, and cancellation retains the exact caller reason.

Executed on current source: 95 tests passed across seven selected files, including both lifetime controls, PE public acceptance, shared descriptor consumer controls, Go CLI/MCP and hash ownership/ADB checks. No source change was needed for this reconciliation. No spontaneous shutdown race, native Windows loader behavior or public transport cancellation reproduction is claimed. This closes the original admitted-parsing lifetime report, not the wider lifecycle audit.

@morluto commented on GitHub (Oct 18, 2026): Reconciled against main94bc7ed3222ab6ca33fe68f58326eefb91689904. The repair and original gated regression were published in effb71f59c01c0cd94fc383ff2e271723750fcf9. Current PeResourcesProvider.inspect admits stable acquisition and the awaited parser inside the same ArtifactResourceScope.run operation. close drains that entire operation and rejects later admission. The checked-in peResourcesLifetime controls use a real authored PE file and real parser, with only parser-entry scheduling gated. Both completion and cancellation controls passed; close stays pending until parsing settles, and cancellation retains the exact caller reason. Executed on current source: 95 tests passed across seven selected files, including both lifetime controls, PE public acceptance, shared descriptor consumer controls, Go CLI/MCP and hash ownership/ADB checks. No source change was needed for this reconciliation. No spontaneous shutdown race, native Windows loader behavior or public transport cancellation reproduction is claimed. This closes the original admitted-parsing lifetime report, not the wider lifecycle audit.

bsakaguchi referenced this issue

2026-10-31 00:41:10 +00:00

[GH-ISSUE #5752] [Docs] IL2CPP native-linking guide omits ambiguous universal-slice refusal #3498

bsakaguchi referenced this issue

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

[GH-ISSUE #5982] [Bug] Unparseable lipo slice output becomes a generic artifact I/O failure #3650

bsakaguchi referenced this issue

2026-10-31 00:55:16 +00:00

[PR #1772] [MERGED] fix(apple): preserve slice identity and macOS boundary diagnostics #4808

bsakaguchi referenced this issue

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

[PR #1982] [MERGED] fix(native): accept universal binaries with LIB64 x86_64 slices #4948