[
https://issues.apache.org/jira/browse/SLING-1571?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Klimetschek updated SLING-1571:
-----------------------------------------
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 ) {
return;
}
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)
was:
QuartzJobExecutor implements a check whether a job is already running if a job
is not allowed to run concurrently:
if (!canRunConcurrently) {
if ( handler.isRunning ) {
return;
}
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 [1] (the first reply).
Fortunately Java 5 comes with java.util.concurrent.atomic.AtomicBoolean, which
has an atomic operation compareAndSet [1]. 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://www.coderanch.com/t/233792/threads/java/Volatile-boolean-versus-AtomicBoolean
[2]
http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/atomic/AtomicBoolean.html#compareAndSet(boolean,%20boolean)
> Scheduler must use AtomicBoolean instead of volatile boolean for
> job-is-running check
> -------------------------------------------------------------------------------------
>
> Key: SLING-1571
> URL: https://issues.apache.org/jira/browse/SLING-1571
> Project: Sling
> Issue Type: Bug
> Components: Commons
> Affects Versions: Commons Scheduler 2.2.0
> Reporter: Alexander Klimetschek
>
> 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 ) {
> return;
> }
> 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)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
