BackburnerJS / backburner.js

A rewrite of the Ember.js run loop as a generic microlibrary
MIT License
392 stars 80 forks source link

Ensure `scheduleOnce` works correctly following a cancelled job #402

Closed davidtaylorhq closed 1 year ago

davidtaylorhq commented 1 year ago

Previously, cancelling a job would splice it from the queue. This was problematic because it affects the index of all later jobs in the queue. The targetQueues map of jobs to queue indexes becomes incorrect, leading to arguments being passed to the wrong jobs. For example, given the sequence:

const toCancel = bb.scheduleOnce('queueName', null, f1, "f1 cancelled schedule");
bb.scheduleOnce('queueName', null, f2, "f2 first schedule");
bb.scheduleOnce('queueName', null, f3, "f3 first schedule");
bb.cancel(toCancel);
bb.scheduleOnce('queueName', null, f2, "f2 second schedule");

The correct behaviour is for f2 to be called with the argument "f2 second schedule", and f3 to be called with "f3 first schedule".

In reality, f2 was being called with the the argument from its first schedule ("f2 first schedule") and f3 was being called with the argument from f2's second schedule ("f2 second schedule").

One possible fix would be to iterate over all targetQueues and decrement them by QUEUE_ITEM_LENGTH if they are greater than the cancelled job's index. That would be O(n) where n is the number of queued jobs. Instead, this commit nulls out out 'method' of the cancelled job in the queue, thereby making it a no-op (O(1)). That is the same technique employed a few lines below when cancelling the job in _queueBeingFlushed.