Skip to content

Make setTimeout and setImmediate timers run consecutively #8460

Description

@bnoordhuis

Continuing from libuv/libuv#1035 (comment):

Node.js uses a uv_idle_t handle to implement setImmediate() timers but I think it could - and should - hang them off the big setTimeout() timer.

Right now you have this weird situation where:

a) setTimeout timers run,
b) some other callbacks run (mostly close callbacks), then
c) setImmediate timers run.

Activity

  1. added
    timersIssues and PRs related to timers, setImmediate(), setInterval(), and setTimeout().
    on Sep 9, 2016
  2. Fishrock123 commented on Sep 10, 2016

    @Fishrock123
    Contributor

    implement setImmediate() timers

    What are these? Just Immediates or timers scheduled in Immediates?

    Immediates should fire after I/O before Timeouts, right?

  3. bnoordhuis commented on Sep 12, 2016

    @bnoordhuis
    MemberAuthor

    Just Immediates or timers scheduled in Immediates?

    Immediates.

    Immediates should fire after I/O before Timeouts, right?

    That's right, the actual callbacks are made from a check handle.

    I realize what tripped me up: we call uv_run(UV_RUN_ONCE) in a loop, causing the big setTimeout timer to run twice per loop tick sometimes.

    The order of events is approximately: start tick -> timeouts -> I/O -> immediates -> close callbacks -> timeouts again -> end tick.

  4. self-assigned this
    on Jan 17, 2017
  5. Trott commented on Jul 15, 2017

    @Trott
    Member

    Should this stay open?

    If so, @Fishrock123: Are you likely to get to this soon? If not, should we unassign you and/or add mentor available and help wanted labels?

  6. Fishrock123 commented on Nov 23, 2017

    @Fishrock123
    Contributor

    I realize what tripped me up: we call uv_run(UV_RUN_ONCE) in a loop, causing the big setTimeout timer to run twice per loop tick sometimes.

    The uv_run(UV_RUN_ONCE) part was changed in 9e08695 ... @bnoordhuis any idea if the ordering issues are still present? (Although we may want to make the change regardless.)

  7. bnoordhuis commented on Nov 23, 2017

    @bnoordhuis
    MemberAuthor

    9e08695 should have fixed the ordering, yes. The issue of timer callbacks and immediate callbacks getting interleaved with other callbacks still exists.

  8. j0t3x commented on Feb 1, 2018

    @j0t3x
    Contributor

    Is this taken? I would like to work on this one.

  9. apapirovski commented on Feb 1, 2018

    @apapirovski
    Contributor

    Hi @j0t3x, unfortunately I don't know if there's a clear direction here even... It's a strong possibility that there might be code out there relying on immediates running after I/O.

    (For reference http://docs.libuv.org/en/v1.x/design.html)

    Also, the fact that setImmediate will run within the same iteration of the loop for everything but close callbacks is IMO a good thing.

  10. j0t3x commented on Feb 7, 2018

    @j0t3x
    Contributor

    The issue of timer callbacks and immediate callbacks getting interleaved with other callbacks still exists.
    @apapirovski how can I help investigate this one?

  11. added
    discussIssues opened for discussion and feedback.
    and removed
    help wantedIssues that need assistance from volunteers or PRs that need help to proceed.
    on Apr 12, 2018
  12. apapirovski commented on Apr 12, 2018

    @apapirovski
    Contributor

    I've removed the help-wanted label. I'm not sure anyone truly knows what's best here.

    I would be in favour of sticking with the current behaviour but of course there is some weirdness associated with it. If we hang them off the timer handle then scheduling them seems to become a bit more problematic — I'm not 100% certain how @bnoordhuis envisions that working, but he also knows libuv a lot better than I so I trust that it's possible in an elegant manner.

  13. bnoordhuis commented on Apr 12, 2018

    @bnoordhuis
    MemberAuthor

    If we hang them off the timer handle then scheduling them seems to become a bit more problematic

    How so?

  14. apapirovski commented on Apr 12, 2018

    @apapirovski
    Contributor

    As far as I can tell they would need to be timers scheduled for 1ms, right? (Scheduling for 0 while in the timer handling process seems to cause those timers to fire again before running the loop again.)

    Or is there some other way to hang them off the timer? As I said, only mildly knowledgeable about libuv.

  15. Fishrock123 commented on Aug 14, 2018

    @Fishrock123
    Contributor

    I'm not sure part of this issue exists anymore, since we no longer use uv_run(UV_RUN_ONCE).

  16. Fishrock123 commented on Aug 14, 2018

    @Fishrock123
    Contributor

    Actually, I'm not sure there is an issue at all anymore.

    The definition of setImmediate is, according to the docs:

    Schedules the "immediate" execution of the callback after I/O events' callbacks.

    The OP mentions that close handles can fire before immediates after timeouts, but the code of uv_run seems to indicate that isn't true: https://github.com/libuv/libuv/blob/7284adfa7a833264fc67d68d39610da27ecbab7c/src/unix/core.c#L350-L399

    The uv loop executes callbacks in the following order each loop:

    1. timers
    2. pending
    3. idle
    4. prepare
    5. anything that goes to i/o polling
    6. check
    7. closing
  17. apapirovski commented on Oct 26, 2018

    @apapirovski
    Contributor

    This should just be closed at this point. There's nothing actionable here, at least nothing that anyone is realistically willing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

discussIssues opened for discussion and feedback.timersIssues and PRs related to timers, setImmediate(), setInterval(), and setTimeout().

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions