queue.idle() is true when empty callback is called.

[Cactusbone]

What version of async are you using?
async 1.5.2

Which environment did the issue occur in (Node version/browser version)
node

What did you do? Please include a minimal reproducable case illustrating issue.

const async = require('async');
const queue = async.queue((task, cb) => {
    setTimeout(() => {
        cb();
    }, 500);
}, 1);

queue.drain = () => {
    console.log("drain", queue.idle(), queue.length(), queue.running());
};

queue.empty = () => {
    console.log("empty", queue.idle(), queue.length(), queue.running());
};

queue.push({item: 1}, () => {
    console.log("item processed!");
});

console.log("push", queue.idle(), queue.length(), queue.running());

What did you expect to happen?
I expected queue.idle() to be false when in the empty callback, since there's something being processed.
docs says empty callback is when the last item from the queue is given to the worker, meaning queue.length() should be zero, but queue.running() should be one. At least, that's how i understand it.

What was the actual result?
queue.idle() was true

push false 1 0
empty true 0 0
done
drain true 0 0

[aearly]

The current behavior is technically correct, because the item you push doesn't begin processing until the next tick of the event loop. Immediately after you push, there are 0 items processing, so the queue is considered idle. Perhaps we should change idle() to reflect pending tasks.

Personally, I would like to remove the deferral of the queue's processing, but doing that would lead to some other unexpected behavior, especially when using priority queues.

[Cactusbone]

I'm using empty to control a stream (pause / resume on saturated/empty), and when the steam ends, I'm checking for idle status on the queue to know if I'm fully done processing or not (if not I'm using drain).
but depending on the steam implementation, I might get the stream end callback in the same tick as the resume call, which was triggered by the queue empty callback.

just to clarify, after the push, the queue is not idle, (first log line), but it's idle during the empty callback.

from the code it seems like the processing does start in the same tick as the empty callback (not right after push):

if (q._tasks.length === 0) {
    q.empty();
}
workers += 1;

just changing workers before calling the empty callback would fix the reported problem, but I don't know about priority queues behaviors.

[aearly]

We cant change the timing of the empty callback without changing the behavior of the queue, e.g. a breaking change. People might be using that callback for other uses.

However, it sounds like what you're doing might be accomplished through a Transform or through stream.

[Cactusbone]

at concurrency=1, a Transform stream does the trick, but i'm using the same code with concurrency=50.

in that case when empty is called, the queue is not idle (it's at 49 concurrent processing tasks, and goes to 50 the next tick)

But it would indeed be a breaking change. right now i'm using a setImmediate to delay the callback, so maybe it's just a documentation thing :)

empty: a callback that is called when the last item from the queue is given to a worker. It's called just before the worker starts processing, so if you want to check for idle, delay until the next tick

[aearly]

After giving this more thought, I think this is indeed a bug. Especially considering the wording in our docs -- it was being called after the task had been shifted off the queue, but not after it was being pushed to the worker list. Changing the timing of empty did not break any of our existing tests, so this exact detail was not specified.

[Cactusbone]

thx :)

[aearly]

Fixed in v2.4.0!