Closed dnwe closed 8 years ago
I think this might introduce a race condition on the good path - as the evaluation of whether runnables.isEmpty() and the setting of running=false are no longer inside the same synchronize statement. So there is a potential window where someone could call put() and the work would not be done.
I think the easy solution is to duplicate the running=false inside the synchronize block that tests runnables.isEmpty(). This doesn't fix the same race for someone throwing Error (or other non-Exception subclass of Throwable). Maybe the code for handling Throwable should also fail any pending work (by walking the promises list and calling setFailure() on the promises) in the same block of code that synchronizes to set running (in the finally block)?
Previously this would cause the promise to remain uncompleted and the WorkList run() to exit with resetting 'running' to false and hence new work added to the list would never be actioned.