views:

140

answers:

5

I have some code that I want to have some one time initialisation performed. But this code doesn't have a definite lifecycle, so my logic can be potentially invoked by multiple threads before my initialisation is done. So, I want to basically ensure that my logic code "waits" until initialisation is done.

This is my first cut.

public class MyClass {
    private static final AtomicBoolean initialised = new AtomicBoolean(false);

    public void initialise() {
        synchronized(initialised) {
            initStuff();
            initialised.getAndSet(true);
            initialised.notifyAll();
        }
    }

    public void doStuff() {
        synchronized(initialised) {
            if (!initialised.get()) {
                try {
                    initialised.wait();
                } catch (InterruptedException ex) {
                    throw new RuntimeException("Uh oh!", ex);
                }
            }
        }

        doOtherStuff();
    }
}

I basically want to make sure this is going to do what I think it's going to do -- block doStuff until the initialised is true, and that I'm not missing a race condition where doStuff might get stuck on a Object.wait() that will never arrive.

Edit:

I have no control over the threads. And I want to be able to control when all of the initialisation is done, which is why doStuff() can't call initialise().

I used an AtomicBoolean as it was a combination of a value holder, and an object I could synchronize. I could have also simply had a "public static final Object lock = new Object();" and a simple boolean flag. AtomicBoolean conveniently gave me both. A Boolean can not be modified.

The CountDownLatch is exactly what I was looking for. I also considered using a Sempahore with 0 permits. But the CountDownLatch is perfect for just this task.

+1  A: 

A synchronized block will automatically block other threads. Just use a simple lock object + status variable:

public class MyClass {
    private static boolean initialised;
    private static final Object lockObject = new Object();

    public void initialise() {
        synchronized (lockObject) {
            if (!initialised) {
                initStuff();
                initialised = true;
            }
        }
    }

    public void doStuff() {
        initialise();
        doOtherStuff();
    }
}
bkail
the boolean doesn't need to be volatile if you're always accessing it from inside a synchronized block.
mdma
Whoops, thanks. I started with double-checked locking + volatile, and then decided it wasn't worth the confusion for this simple scenario. Fixed even though another answer was accepted.
bkail
A: 

You're using AtomicBoolean always from inside a synchronized block. There's not much point to that since only one thread can access it. Atomic variables are intended for use in lock-free solutions - you can get and set the value as an uninterruptable unit.

I guess you are looking for a lock free solution once the intiialization has happened:

public class MyClass {
    private static final AtomicBoolean initialised = new AtomicBoolean(false);

    public void initialise() {
        if (!intialized.get())
        {
            synchornized (this)
            {
               if (!initialized.getAndSet(true))
                  doInitialize();
            }
        }
    }

    public void doStuff() {
        initialize();
        doOtherStuff();
    }

You could also do this with a simple volatile boolean which is actually a little more efficient than an AtomicBoolean.

mdma
A: 

It this is right at startup, why not wait to start the other threads until the initialization is complete?

Also, you can do a thread-synchronized IsComplete boolean that is set to false until it is set to true by the initialization routine.

uosɐſ
+6  A: 

That's a strange mix of library and built-in concurrency controls. Something like this is much cleaner:

public class MyClass {

  private static final CountDownLatch latch = new CountDownLatch(1);

  public void initialise() {
    initStuff();
    latch.countDown();
  }

  public void doStuff() {
    try {
      latch.await();
    } catch (InterruptedException ex) {
      throw new RuntimeException("Uh oh!", ex);
    }
    doOtherStuff();
  }

}
erickson
+1 I didn't know about that one, but it seems pretty useful
karoberts
does a CountDownLatch require synchronization/locking when it reaches zero?
mdma
+1 - CountDownLatch.await doesn't block when it reaches zero - just checked the javadocs.
mdma
@mdma - I believe it is equivalent to a `volatile` read.
erickson
@erickson - True, I just checked the source code. After the count is zero, await() calls Thread.interrupted() and does one volatile read.
Esko Luontola
+1  A: 

The best may be to use a static initializer (as mentioned by SB):

public class MyClass {

    public static void doInitialize() {
      ...
    }

    public void doStuff() {
        doOtherStuff();
    }

    static {
       doInitialize();
    }
}

This will get executed once before any other code is allowed to be called. If you will always have to initialize anytime the class is used then there is no performance hit as the class will not be loaded until it is used. See the answers to this question for more details.

Kathy Van Stone