electron/node

Possible issues with on_watcher_queue_updated callback

ckerr opened this issue · 1 comments

ckerr commented

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?

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.