Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

Possible issues with on_watcher_queue_updated callback #45

Open

Description

@ckerr

When looking at this code with @codebytere for us to make a PR to propose upstream, we found a couple of things that we weren't sure about and would like another opinion on:

  1. Most of the 'queue updated' calls work like this:
void uv__io_start(uv_loop_t* loop, uv__io_t* w, unsigned int events) {
  ...
  else if (QUEUE_EMPTY(&w->watcher_queue)) {
    QUEUE_INSERT_TAIL(&loop->watcher_queue, &w->watcher_queue);
    if (loop->on_watcher_queue_updated)
      loop->on_watcher_queue_updated(loop);
  }

But through the call chain of uv_loop_init() -> uv_async_init() -> uv__async_start() -> uv__io_start(), any loop already has an async watcher on it as soon as it's initialized. So if I'm reading this right, the callback will never be triggered for the async watcher because we'll never have an empty queue and a non-NULLon_queue_watcher_updated

  1. One of the callback triggers doesn't look like the others. uv__io_feed() pumps it unconditionally, regardless of whether the loop->watcher_queue changes:
void uv__io_feed(uv_loop_t* loop, uv__io_t* w) {
  if (QUEUE_EMPTY(&w->pending_queue))
    QUEUE_INSERT_TAIL(&loop->pending_queue, &w->pending_queue);
  if (loop->on_watcher_queue_updated)
    loop->on_watcher_queue_updated(loop);
}

Are these intentional? If so, could you walk us through it so that we can better understand this code?

Activity

  1. zcbenz commented on Jul 30, 2018

    @zcbenz
    Contributor

    So if I'm reading this right, the callback will never be triggered for the async watcher because we'll never have an empty queue and a non-NULLon_queue_watcher_updated

    We have a few tasks pending immediately after initializing the loop, it does not matter if we missed one update at this stage.

    Are these intentional?

    It is not intentional, I put the code there and it worked fine so I did not do more improvements.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions