You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Carsten Ziegeler (JIRA)" <ji...@apache.org> on 2010/07/05 08:06:50 UTC

[jira] Resolved: (SLING-1571) Scheduler must use AtomicBoolean instead of volatile boolean for job-is-running check

     [ https://issues.apache.org/jira/browse/SLING-1571?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Carsten Ziegeler resolved SLING-1571.
-------------------------------------

    Resolution: Fixed

Thanks for reporting this - it's fixed now in revision 960452

> 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
>            Assignee: Carsten Ziegeler
>             Fix For: Commons Scheduler 2.2.2
>
>
> 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.