Skip to content

fix(ext/node): add _idleStart and _idleTimeout to Timeout#33604

Merged
bartlomieju merged 2 commits into
mainfrom
fix/timer-idle-start-timeout
Apr 27, 2026
Merged

fix(ext/node): add _idleStart and _idleTimeout to Timeout#33604
bartlomieju merged 2 commits into
mainfrom
fix/timer-idle-start-timeout

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

Summary

  • Adds _idleStart property to Timeout objects, set to Date.now() on creation, refresh, and destroy — matching Node.js behavior
  • Sets _idleTimeout = -1 in kDestroy to signal a cleared timer, matching Node.js's unenroll semantics
  • Enables parallel/test-tls-wrap-timeout.js Node.js compat test

Test plan

  • ./x test-compat test-tls-wrap-timeout.js passes

….js compat

Node.js Timeout objects expose `_idleStart` (set to current time on
creation and refresh) and set `_idleTimeout = -1` when destroyed. Deno
was missing both, causing tests that inspect timer internals to fail.

Enables `parallel/test-tls-wrap-timeout.js` Node.js compat test.
Copy link
Copy Markdown
Contributor

@fibibot fibibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three changes map directly to what the upstream test asserts:

  • Timeout() constructor sets _idleStart = DateNow() ✓ — matches the test's initial lastIdleStart = socket[kTimeout]._idleStart capture.
  • refresh() updates _idleStart ✓ — Node's documented behavior.
  • kDestroy sets both _idleTimeout = -1 and _idleStart = DateNow() ✓ — both required by the test's exit-time assertions: socket[kTimeout]._idleTimeout === -1 AND lastIdleStart < socket[kTimeout]._idleStart.

DateNow correctly added to the primordials destructure. Test enrollment alphabetically positioned (wrap-event-emmiter < wrap-timeout < zero-clear-in).

CI is still building (44/110 passing, no failures yet). Holding off on APPROVE until the relevant test node_compat debug and test unit_node debug jobs land green per fibibot's CI gate. I'll re-confirm once they finish.

@bartlomieju bartlomieju enabled auto-merge (squash) April 27, 2026 20:08
@bartlomieju bartlomieju merged commit 134f7d7 into main Apr 27, 2026
136 checks passed
@bartlomieju bartlomieju deleted the fix/timer-idle-start-timeout branch April 27, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants