Reputation: 319

Signalling condition variable in Java to trigger await

I created a Runnable class which periodically does some task. This class has a method to trigger immediate performance of a task discarding whether specified time has elapsed or not.

Class do perform periodic tasks in specified time but triggering does not work as expected.
Below is simplified class code with main method. According to API docs in my interpretation, when trigger is called, await is signaled and it has to return so that task is performed more or less immediately without waiting elapse of specified time.

What do I get wrong with Java locks and conditions? I could not figure out why my trigger method does not behave as expected?

import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.*;

public class PeriodicRunner implements Runnable {

    private Lock lock = new ReentrantLock();
    private Condition cond = lock.newCondition();

    public void trigger() {
        lock.lock();
        try {
            cond.signalAll();
        }
        finally {
            lock.unlock();
        }
    }

    @Override
    public void run() {
        lock.lock();
        try {
            while(true) {
                cond.await(5, TimeUnit.SECONDS);
                System.out.println("some task here");
            }
        }
        catch(InterruptedException ex) {
        }
        finally {
            lock.unlock();
        }
    }

    public static void main(String[] args) throws InterruptedException {

        PeriodicRunner pr = new PeriodicRunner();
        new Thread(pr).start();
        pr.trigger();
    }
}

Upvotes: 1

Answers (3)

SimonC

Reputation: 6718

I think what you want could be achieved more transparently with a ScheduledExecutorService:

class PeriodicRunner implements Runnable
{
  private final ScheduledExecutorService _scheduler = Executors.newSingleThreadScheduledExecutor();

  PeriodicRunner() 
  {
    _scheduler.scheduleWithFixedDelay(this, 5, 5, TimeUnit.SECONDS);
  }

  public void trigger()
  {
    _scheduler.execute(this);
  }

  public void run()
  {
    System.out.println("some task here");
  }
}

UPDATE

For example, the following:

PeriodicRunner runner = new PeriodicRunner();
Thread.sleep(TimeUnit.SECONDS.toMillis(12));
System.out.println("[" + new Date() + "] triggering extra task");
runner.trigger();    
Thread.sleep(TimeUnit.SECONDS.toMillis(1));
System.out.println("[" + new Date() + "] triggering extra task");
runner.trigger();
Thread.sleep(TimeUnit.SECONDS.toMillis(1));
System.out.println("[" + new Date() + "] triggering extra task");
runner.trigger();

Would produce the output:

[Wed Mar 20 13:18:31 CST 2013] some task here
[Wed Mar 20 13:18:36 CST 2013] some task here
[Wed Mar 20 13:18:38 CST 2013] triggering extra task
[Wed Mar 20 13:18:38 CST 2013] some task here
[Wed Mar 20 13:18:39 CST 2013] triggering extra task
[Wed Mar 20 13:18:39 CST 2013] some task here
[Wed Mar 20 13:18:40 CST 2013] triggering extra task
[Wed Mar 20 13:18:40 CST 2013] some task here
[Wed Mar 20 13:18:41 CST 2013] some task here
[Wed Mar 20 13:18:46 CST 2013] some task here

UPDATE 2

To expand on d3rzKy's answer, to fix your existing code, you could introduce a flag to indicate that trigger has been called. Also, await isn't guaranteed to wait for the entire time, so you need to loop and keep track of how long you have already waited for.

import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.*;

public class PeriodicRunner implements Runnable {

    private final Lock lock = new ReentrantLock();
    private final Condition cond = lock.newCondition();

    private volatile boolean triggered = false;

    public void trigger() {
        lock.lock();
        try {
            triggered = true;
            cond.signalAll();
        }
        finally {
            lock.unlock();
        }
    }

    @Override
    public void run() {
        lock.lock();
        try {
            while(true) {

                if (!triggered) {
                    long remainingMs = TimeUnit.SECONDS.toMillis(5);
                    long dueTimeMs = System.currentTimeMillis() + remainingMs;
                    while (remainingMs > 0 && !triggered) {
                        if (cond.await(remainingMs, TimeUnit.MILLISECONDS)) {
                            break;
                        }
                        remainingMs = dueTimeMs - System.currentTimeMillis();
                    }
                }
                triggered = false;
                System.out.println("some task here");
            }
        }
        catch(InterruptedException ex) {
        }
        finally {
            lock.unlock();
        }
    }

    public static void main(String[] args) throws InterruptedException {

        PeriodicRunner pr = new PeriodicRunner();
        new Thread(pr).start();
        pr.trigger();
    }
}

Upvotes: 2