morluto/rea

Fork 0

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

[GH-ISSUE #2539] fix(inspector): release the observation timer after an early disconnect #856

New issue

Closed

opened 2026-10-15 18:29:02 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:29:02 +00:00

Owner

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

Summary

An Inspector observation leaves its duration timer pending after the target disconnects and the call returns an error. A long requested window therefore retains an otherwise unused timer for its remaining duration. The native timer remains referenced and can delay process exit after a one-shot invocation.

Steps to reproduce

Start a disposable loopback HTTP/WebSocket fixture on port 9230. /json/version returns { "Browser": "node.js/v24.4.1", "Protocol-Version": "1.3" }. /json/list returns one target with id: "target", type: "node", url: "https://example.test/app.js", attached: false and webSocketDebuggerUrl: "ws://127.0.0.1:9230/target".
Accept /target, reply to both Runtime.enable and Debugger.enable with { "id": <command id>, "result": {} }, and close the socket 30 ms after the second reply.
In a focused integration test, wrap the native setTimeout and clearTimeout functions to record timer handles, delays, callbacks and clear calls while preserving their behavior. Call new V8InspectorProvider().observe({ inspector_endpoint: "http://127.0.0.1:9230", target_id: "target", observation_ms: 60000 }) without a cancellation signal.
After the error returns, inspect the recorded 60000-ms timer. Check whether its callback fired, whether it was cleared, and the native handle's hasRef() value. Clear the test-owned timer after recording the result so the probe does not wait a minute.
As a control, keep the socket open and request a 40-ms window. Verify that observation succeeds and its duration timer fires before the call returns.

Expected behavior

When an observation ends because its target disconnects, the provider returns the disconnection error and releases its local duration timer. A successful observation still waits for the caller's requested window. Finishing the operation does not require aborting the caller's cancellation signal.

Actual behavior

The provider returned disconnected with a partial observation, but its 60000-ms timer remained unfired, uncleared and referenced. The native failure diagnostic also showed that the timer was not destroyed. The 40-ms control returned success after its timer fired. The two provider calls ran in one focused test; the cleanup assertion failed. A standalone CLI process was not executed; the process-exit consequence follows from the retained referenced native timer.

Evidence

Expected source: AGENTS.md requires resource ownership and safe resource bounds. src/browser/execution/CdpExecutionWindow.ts:105 specifies local timer release on every setup/failure path, and src/browser/execution/CdpExecutionCollection.ts:58 specifies timer removal on success, cancellation or failure for the analogous observation lifecycle.
Failure source: src/inspector/V8InspectorProvider.ts:512 races its duration wait with disconnect and removes only the disconnect subscription. src/browser/CdpCaptureValues.ts:104 clears the real delay timer only on signal abort. The focused provider fixture directly recorded the surviving referenced timer after the error result.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

Restoration check

Repeat the early-disconnect fixture with the long window. Receive the same disconnection error and partial observation, with no live duration timer left behind. Retain the open-socket control and its full selected wait so cleanup does not shorten a successful capture. Also verify cleanup with a present but un-aborted caller signal without altering that signal.

Runtime or environment

Source baseline 8b181716ac. Node v26.10.0, npm 11.19.1 and lockfile-installed dependencies. Verification used npm run test:focused, a disposable loopback fixture and real native timers with recording wrappers.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Every tested early-disconnect call retained its requested duration timer after returning, creating unnecessary resource retention and a referenced timer that can delay process exit. One early-disconnect call and one successful control were observed.
Workaround status: unverified
Workaround basis: Shorter windows change the caller's requested observation duration. Provider consumers may be able to release the delay by aborting a supplied signal, but that recovery was not tested and is not exposed as a post-return cleanup option by the one-shot command.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2539 ## Summary An Inspector observation leaves its duration timer pending after the target disconnects and the call returns an error. A long requested window therefore retains an otherwise unused timer for its remaining duration. The native timer remains referenced and can delay process exit after a one-shot invocation. ## Steps to reproduce 1. Start a disposable loopback HTTP/WebSocket fixture on port 9230. `/json/version` returns `{ "Browser": "node.js/v24.4.1", "Protocol-Version": "1.3" }`. `/json/list` returns one target with `id: "target"`, `type: "node"`, `url: "https://example.test/app.js"`, `attached: false` and `webSocketDebuggerUrl: "ws://127.0.0.1:9230/target"`. 2. Accept `/target`, reply to both `Runtime.enable` and `Debugger.enable` with `{ "id": <command id>, "result": {} }`, and close the socket 30 ms after the second reply. 3. In a focused integration test, wrap the native `setTimeout` and `clearTimeout` functions to record timer handles, delays, callbacks and clear calls while preserving their behavior. Call `new V8InspectorProvider().observe({ inspector_endpoint: "http://127.0.0.1:9230", target_id: "target", observation_ms: 60000 })` without a cancellation signal. 4. After the error returns, inspect the recorded 60000-ms timer. Check whether its callback fired, whether it was cleared, and the native handle's `hasRef()` value. Clear the test-owned timer after recording the result so the probe does not wait a minute. 5. As a control, keep the socket open and request a 40-ms window. Verify that observation succeeds and its duration timer fires before the call returns. ## Expected behavior When an observation ends because its target disconnects, the provider returns the disconnection error and releases its local duration timer. A successful observation still waits for the caller's requested window. Finishing the operation does not require aborting the caller's cancellation signal. ## Actual behavior The provider returned `disconnected` with a partial observation, but its 60000-ms timer remained unfired, uncleared and referenced. The native failure diagnostic also showed that the timer was not destroyed. The 40-ms control returned success after its timer fired. The two provider calls ran in one focused test; the cleanup assertion failed. A standalone CLI process was not executed; the process-exit consequence follows from the retained referenced native timer. ## Evidence - Expected source: `AGENTS.md` requires resource ownership and safe resource bounds. `src/browser/execution/CdpExecutionWindow.ts:105` specifies local timer release on every setup/failure path, and `src/browser/execution/CdpExecutionCollection.ts:58` specifies timer removal on success, cancellation or failure for the analogous observation lifecycle. - Failure source: `src/inspector/V8InspectorProvider.ts:512` races its duration wait with disconnect and removes only the disconnect subscription. `src/browser/CdpCaptureValues.ts:104` clears the real delay timer only on signal abort. The focused provider fixture directly recorded the surviving referenced timer after the error result. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete ## Restoration check Repeat the early-disconnect fixture with the long window. Receive the same disconnection error and partial observation, with no live duration timer left behind. Retain the open-socket control and its full selected wait so cleanup does not shorten a successful capture. Also verify cleanup with a present but un-aborted caller signal without altering that signal. ## Runtime or environment Source baseline 8b181716aca2cb467be499cba226abfb5ae19141. Node v26.10.0, npm 11.19.1 and lockfile-installed dependencies. Verification used `npm run test:focused`, a disposable loopback fixture and real native timers with recording wrappers. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Every tested early-disconnect call retained its requested duration timer after returning, creating unnecessary resource retention and a referenced timer that can delay process exit. One early-disconnect call and one successful control were observed. - Workaround status: unverified - Workaround basis: Shorter windows change the caller's requested observation duration. Provider consumers may be able to release the delay by aborting a supplied signal, but that recovery was not tested and is not exposed as a post-return cleanup option by the one-shot command.

bsakaguchi

2026-10-15 18:29:02 +00:00

closed this issue
added the
area: domain

bug
labels

bsakaguchi commented

2026-10-15 18:29:03 +00:00

Author

Owner

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

Addressed on main in 55778f20bacffe428e2cddcff81ffe01c0d477ae.

Early capture completion/disconnect cancels the wait's owned timer without aborting the caller's signal.

Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit.

Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

@morluto commented on GitHub (Oct 14, 2026): Addressed on main in [55778f20bacffe428e2cddcff81ffe01c0d477ae](https://github.com/morluto/rea/commit/55778f20bacffe428e2cddcff81ffe01c0d477ae). Early capture completion/disconnect cancels the wait's owned timer without aborting the caller's signal. Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit. Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

bsakaguchi referenced this issue

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

[PR #856] [MERGED] refactor(ghidra): extract createClient and load-image attestation #2293

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

No description provided.

Rows
Columns