Details
-
Bug
-
Status: Closed
-
Major
-
Resolution: Fixed
-
Commons Scheduler 2.2.0
-
None
Description
QuartzJobExecutor [1] implements a check whether a job is already running if a job is not allowed to run concurrently:
if (!canRunConcurrently) {
if ( handler.isRunning )
handler.isRunning = true;
}
handler.isRunning is declared as "public volatile boolean".
This will fail if isRunning is false and two jobs start at exactly the same time - both will see a value of false and start. The problem is that "volatile" doesn't make the two commands "isRunning + set to true" atomic. It just makes sure that updates can be read immediately by all threads, ie. it would only help to see a change of the variable, done by another thread, more quickly. It doesn't help in this situation. See also [2] (the first reply).
Fortunately Java 5 comes with java.util.concurrent.atomic.AtomicBoolean, which has an atomic operation compareAndSet [3]. Code would probably look like this:
public AtomicBoolean isRunning;
....
if (!canRunConcurrently) {
if ( !handler.isRunning.compareAndSet(false, true) ) { return; }
}
This is probably not so critical, since I guess it's very unlikely that the scheduler triggers a job execution twice at the same time in the first place, but you'll never know.
[1] http://svn.apache.org/repos/asf/sling/trunk/bundles/commons/scheduler/src/main/java/org/apache/sling/commons/scheduler/impl/QuartzJobExecutor.java
[2] http://www.coderanch.com/t/233792/threads/java/Volatile-boolean-versus-AtomicBoolean
[3] http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/atomic/AtomicBoolean.html#compareAndSet(boolean,%20boolean)