Skip to content

fix: Timer execution order#969

Merged
richarddavison merged 1 commit intomainfrom
fix/timer-execution-order
May 5, 2025
Merged

fix: Timer execution order#969
richarddavison merged 1 commit intomainfrom
fix/timer-execution-order

Conversation

@richarddavison
Copy link
Copy Markdown
Collaborator

@richarddavison richarddavison commented May 3, 2025

Issue # (if available)

Fixes: #965

Description of changes

Timers are batched for performance reasons. A timer batch was not sorted. This PR sorts timers according to their deadline.

Checklist

  • Created unit tests in tests/unit and/or in Rust for my feature if needed
  • Ran make fix to format JS and apply Clippy auto fixes
  • Made sure my code didn't add any additional warnings: make check
  • Added relevant type info in types/ directory
  • Updated documentation if needed (API.md/README.md/Other)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@nabetti1720
Copy link
Copy Markdown
Contributor

nabetti1720 commented May 3, 2025 •
edited
Loading

@richarddavison Thank you for the quick fix!
I tried applying this to a branch I was developing (It's just for verification purposes), and made an additional improvement by setting the deadline of setImmedate() to one second before the current date and time, and confirmed that it was running as expected.

% llrt blockingTimer.js
🦖 [1] MAINLINE: Sync process: start
🦖 [2] MAINLINE: Sync process: timer exit after 2000
🦖 [3] MAINLINE: Sync process: end
👶 [4] MICRO: queueMicrotask
👨 [5] MACRO: setImmediate
⏰ [6] TIMER: setTimeout[0ms] finished
⏰ [7] TIMER: setTimeout[500ms] finished
⏰ [8] TIMER: setTimeout[1000ms] finished

@nabetti1720 nabetti1720 mentioned this pull request May 3, 2025
Merged
5 tasks
@richarddavison richarddavison merged commit 1341cb3 into main May 5, 2025
11 checks passed
@richarddavison richarddavison deleted the fix/timer-execution-order branch May 5, 2025 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The execution order of asynchronous processes differs from other runtimes

2 participants

@richarddavison @nabetti1720