views:

347

answers:

5

I have a class proposing translations utilities. The translations themselves should be reloaded every 30 minutes. I use Spring Timer support for that. Basically, my class looks like :

public interface Translator {
    public void loadTranslations();
    public String getTranslation(String key);
}

loadTranslations() can be pretty long to run, so while it is running the old translations are still available. This is done by loading the translations in a local Map and just changing the reference when all translations are loaded.

My problem is : how do I make sure that when a thread is already loading translations, is a second one also tries to run, it detects that and returns immediately, without starting a second update.

A synchronized method will only queue the loads ... I'm still on Java 1.4, so no java.util.concurrent.

Thanks for your help !

A: 

public synchronized void loadTranslations();

will stop multiple Threads calling loadTranslations at the same time, the Thread that called loadTranslations when another Thread is in the synchronized will only enter loadTranslations after the other Thread has left the synchronized block (in this case the method is the syncronized block).

You could keep a static last updated time so you can decide if you want to not bother with another loadTranslations() call too soon after the last call.

Paul Whelan
A: 

Keep a handle on the load thread to see if it's running?

Or can't you just use a synchronized flag to indicate if a load is in progress?

cagcowboy
+1  A: 

I am from a .net background(no java experience at all), but you could try a simple static flag of some sort that checks at the beginning of the method if its alrady running. Then all you need to do is make sure any read/write of that flag is synchronized. So at beginning check the flag, if its not set, set it, if it is set, return. If its not set, run the rest of the method, and after its complete, unset it. Just make sure to put the code in a try/finally and the flag iunsetting in the finally so it always gets unset in case of error. Very simplified but may be all you need.

Edit: This actually probably works better than synchronizing the method. Because do you really need a new translation immediately after the one before it finishes? And you may not want to lock up a thread for too long if it has to wait a while.

mattlant
This goes in the right direction, but the synchronization is not as easy as synchronizing the read and the writes. The check of the field and its writing have to be atomic ...
Guillaume
A: 

This is actually identical to the code that is required to manage the construction of a Singleton (gasp!) when done the classical way:

if (instance == null) {
  synchronized {
    if (instance == null) {
       instance = new SomeClass();
    }
  }
}

The inner test is identical to the outer test. The outer test is so that we dont routinely enter a synchronised block, the inner test is to confirm that the situation has not changed since we last made the test (the thread could have been preempted before entering Synchronized).

In your case:

if (translationsNeedLoading()) {
  synchronized {
    if (translationsNeedLoading()) {
       loadTranslations();
    }
  }
}

UPDATE: This way of constructing a singleton will not work reliably under your JDK1.4. For explanation see here. However I think you are you will be OK in this scenario.

Ewan Makepeace
This code is not safe (as you point out in the link you gave). And it will queue the callers, which is exactly what I am trying to prevent.
Guillaume
+2  A: 

Use some form of locking mechanism to only perform the task if it is not already in progress. Acquiring the locking token must be a one-step process. See:

/**
 * @author McDowell
 */
public abstract class NonconcurrentTask implements Runnable {

    private boolean token = true;

    private synchronized boolean acquire() {
     boolean ret = token;
     token = false;
     return ret;
    }

    private synchronized void release() {
     token = true;
    }

    public final void run() {
     if (acquire()) {
      try {
       doTask();
      } finally {
       release();
      }
     }
    }

    protected abstract void doTask();

}

Test code that will throw an exception if the task runs concurrently:

public class Test {

    public static void main(String[] args) {
     final NonconcurrentTask shared = new NonconcurrentTask() {
      private boolean working = false;

      protected void doTask() {
       System.out.println("Working: "
         + Thread.currentThread().getName());
       if (working) {
        throw new IllegalStateException();
       }
       working = true;
       try {
        Thread.sleep(1000);
       } catch (InterruptedException e) {
        throw new RuntimeException(e);
       }
       if (!working) {
        throw new IllegalStateException();
       }
       working = false;
      }
     };

     Runnable taskWrapper = new Runnable() {
      public void run() {
       while (true) {
        try {
         Thread.sleep(100);
        } catch (InterruptedException e) {
         throw new RuntimeException(e);
        }
        shared.run();
       }
      }
     };
     for (int i = 0; i < 100; i++) {
      new Thread(taskWrapper).start();
     }
    }

}
McDowell