Mitigate workQueue race condition (#240)

[colinmccabe]

• Move workQueue length check out of setTimeout
• Quickly store a reference to workQueue[0] in a local variable in case workQueue[0] changes

Running Async tasks more frequently or increasing the setTimeout delay in onComplete increases the frequency of the exception described in #240, which suggests that the bug is indeed caused by workQueue[0] being overwritten with a proceeding Async task before runTask() can run.

I haven't observed the exception with this patch in place, even under extreme conditions with Async tasks being run one after another, milliseconds apart. Without the patch, there is a blizzard of exceptions.

Is this enough to fix the race condition? Not sure exactly how async JS works.

[evancz]

Great sleuthing!

Based on this, I think the current issue comes from a bad interaction between onComplete and register. Before, it was possible for a task to complete in onComplete bringing the number of active tasks down to zero. Then register would add a task and start running it. Then the setTimeout would run, checking if there's more work to do, which now there is, and then starts running the first task again. This repeats all the work until it gets to where the first run paused, causing the crash.

That's my understanding at least. And under that theory, your fix should avoid this.

[evancz]

Great sleuthing!

Based on this, I think the current issue comes from a bad interaction between onComplete and register. Before, it was possible for a task to complete in onComplete bringing the number of active tasks down to zero. Then register would add a task and start running it. Then the setTimeout would run, checking if there's more work to do, which now there is, and then starts running the first task again. This repeats all the work until it gets to where the first run paused, causing the crash.

That's my understanding at least. And under that theory, your fix should avoid this.