[PR #2793] [CLOSED] fix(process): re-arm the startup deadline timer instead of aborting on early delivery #5398

Closed
opened 2026-10-31 01:06:19 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2793
Author: @nosmile99
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/process-startup-deadline-early-timer-abort


📝 Commits (4)

  • 3e1a366 fix(process): re-arm the startup deadline timer instead of aborting on early delivery
  • 5460016 fix(process): keep full-budget wait pending when both deadline timers fire early
  • a355832 test(process): cover wait expiry before deadline callback
  • 3a1faf4 fix: reconcile captured main and owned findings for PR #2793

📊 Changes

1 file changed (+42 additions, -0 deletions)

View changed files

📝 src/process/ProviderDeadline.test.ts (+42 -0)

📄 Description

Fixes #2760.

Problem

Node timers can fire up to ~1ms early. ProviderDeadline's constructor set the setTimeout callback to unconditionally record #interruption = "timeout" and abort the shared signal on fire. So a 40ms deadline could abort at monotonic 39ms while remainingMs() (via Math.ceil) still reported 1ms — the deadline simultaneously claimed timeout and 1ms remaining. Consumers (GhidraClient, GhidraSocketConnection, HopperClient, OwnedCommand) share this state, so a provider handshake could be aborted before its monotonic budget expired.

Fix

  • The timer callback now re-checks performance.now() >= #deadlineAt before classifying the event as a timeout. On early delivery it re-arms for the residual remainingMs() (strictly decreasing, so it converges) instead of aborting a still-future deadline.
  • A wait() clamped to the full budget that resolves exactly at the deadline now reports "aborted" (monotonic check) instead of racing the re-armed timer; short polling intervals still resolve "elapsed". Caller-cancellation classification and budget reporting are untouched, per the issue's constraints.

How this differs from #2720

#2720 takes the masking approach (force remainingMs() to 0 after the timer fires) and leaves the early abort in place; #2760 explicitly prescribes the re-arm correction instead. This PR implements the issue author's requirements. If #2720 merges first, happy to rebase/rework onto whatever semantics maintainers choose.

Evidence

  • Regression: 6 new deterministic fake-timer tests in src/process/ProviderDeadline.test.ts covering the issue's required cases (early delivery, exact expiry, re-arm convergence, wait-at-deadline, sub-budget polling, caller cancellation, dispose cleanup). 3 of 8 fail pre-fix (early-delivery aborts, re-arm, wait-at-deadline); 8/8 pass post-fix, stable across 6 repeat runs.
  • Consumer suites: ProviderDeadline + ProviderProcessOutput + GhidraProvider.behavior + HopperProvider = 63/63 green.
  • npm run check:fast (typecheck + lint) green; verify:test-discovery (1057 files) passes.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2793 **Author:** [@nosmile99](https://github.com/nosmile99) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/process-startup-deadline-early-timer-abort` --- ### 📝 Commits (4) - [`3e1a366`](https://github.com/morluto/rea/commit/3e1a366340f2eae82b17e14694a0a70f31746410) fix(process): re-arm the startup deadline timer instead of aborting on early delivery - [`5460016`](https://github.com/morluto/rea/commit/546001693636f13700d86c6e93b6086abba2b7d6) fix(process): keep full-budget wait pending when both deadline timers fire early - [`a355832`](https://github.com/morluto/rea/commit/a355832fa46c4a907fb292e4370d073aef9fafdc) test(process): cover wait expiry before deadline callback - [`3a1faf4`](https://github.com/morluto/rea/commit/3a1faf46c429862a73d944801fdc6c14b57b21c8) fix: reconcile captured main and owned findings for PR #2793 ### 📊 Changes **1 file changed** (+42 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/process/ProviderDeadline.test.ts` (+42 -0) </details> ### 📄 Description Fixes #2760. ## Problem Node timers can fire up to ~1ms early. `ProviderDeadline`'s constructor set the `setTimeout` callback to unconditionally record `#interruption = "timeout"` and abort the shared signal on fire. So a 40ms deadline could abort at monotonic 39ms while `remainingMs()` (via `Math.ceil`) still reported 1ms — the deadline simultaneously claimed *timeout* and *1ms remaining*. Consumers (`GhidraClient`, `GhidraSocketConnection`, `HopperClient`, `OwnedCommand`) share this state, so a provider handshake could be aborted before its monotonic budget expired. ## Fix - The timer callback now re-checks `performance.now() >= #deadlineAt` before classifying the event as a timeout. On early delivery it re-arms for the residual `remainingMs()` (strictly decreasing, so it converges) instead of aborting a still-future deadline. - A `wait()` clamped to the full budget that resolves exactly at the deadline now reports `"aborted"` (monotonic check) instead of racing the re-armed timer; short polling intervals still resolve `"elapsed"`. Caller-cancellation classification and budget reporting are untouched, per the issue's constraints. ## How this differs from #2720 #2720 takes the masking approach (force `remainingMs()` to 0 after the timer fires) and leaves the early abort in place; #2760 explicitly prescribes the re-arm correction instead. This PR implements the issue author's requirements. If #2720 merges first, happy to rebase/rework onto whatever semantics maintainers choose. ## Evidence - Regression: 6 new deterministic fake-timer tests in `src/process/ProviderDeadline.test.ts` covering the issue's required cases (early delivery, exact expiry, re-arm convergence, wait-at-deadline, sub-budget polling, caller cancellation, dispose cleanup). **3 of 8 fail pre-fix** (early-delivery aborts, re-arm, wait-at-deadline); **8/8 pass post-fix**, stable across 6 repeat runs. - Consumer suites: ProviderDeadline + ProviderProcessOutput + GhidraProvider.behavior + HopperProvider = **63/63 green**. - `npm run check:fast` (typecheck + lint) green; `verify:test-discovery` (1057 files) passes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:06:19 +00:00
Sign in to join this conversation.
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#5398
No description provided.