Static initializer and static synchronized method locking issue

I ran into some locking issue in my application, in which contains several classes like below:

public interface AppClient {
    void hello();
}

public class Client implements AppClient {
    public synchronized static AppClient getInstance() {
        return instance;
    }

    public void hello() {
        System.out.println("Hello Client");
    }

    private final static class InnerClient implements AppClient {
        public void hello() {
            System.out.println("Hello InnerClient");
        }
    }
    private static AppClient instance;

    static {
        instance = new InnerClient();
        doSomethingThatWillCallClientGetInstanceSeveralTimes();
    }
}

public class Application {
    new Thread() {
        AppClient c = Client.getInstance();
        c.hello();
    }.start();
    new Thread() {
        AppClient c = Client.getInstance();
        c.hello();
    }.start();
    // ...
    new Thread() {
        AppClient c = Client.getInstance();
        c.hello();
    }.start();
}

In the doSomethingThatWillCallClientGetInstanceSeveralTimes() method, it will do quite a lot initialization work involving many classes and circularly call Client.getInstance static method several times during the initialization (I understand this is not good, however, this is a legacy code base which lasts for 20+ years).

Here is my problem:

1) I thought before the class Client i开发者_开发技巧nitialization completes, only the first thread that triggers the class Client initialization can access Client.getInstance method because JVM will synchronize on the Client.class object before the class initialization completes. I read the JLS on related topic and came to this conclusion (section 12.4.2, Detailed Initialization Procedure, http://java.sun.com/docs/books/jls/third_edition/html/execution.html).

2) However, it was not the behavior as I saw in my real environment. For example, there are three threads calling Client.getInstance(), thread-1 triggers the Client.class initialization, and calls Client.getInstance() in doSomethingThatWillCallClientGetInstanceSeveralTimes() method several times. And before the completion of doSomethingThatWillCallClientGetInstanceSeveralTimes() method, thread-2 acquires the lock of Client.class object (how is that possible? but it did happen), and enters into Client.getInstance method (because this method is a static synchronized method). For some reason, thread-2 cannot return the "instance" (I guess it is waiting for Client.class completes its initialization). At the same time, thread-1 cannot proceed because it still needs to call Client.getInstance in doSomethingThatWillCallClientGetInstanceSeveralTimes() and cannot acquire the lock since it is owned by thread-2. Threaddump tells me that thread-2 is in RUNNABLE state, and thread-1 is in BLOCKED state waiting for the lock owned by thread-2.

I can only reproduce this behavior in 64 bit Java 6u23 JVM in Windows and cannot reproduce it a 32 bit Java 6 JVM + Windows environment. Can someone tell me what am I missing here? Is this kind of code doomed to give rise to such locking, if yes, how come? Is my understanding on JLS for this part is incorrect? Or it is a JVM issue? Any help is appreciated. Thanks.

To me that looks like a bug. While one thread is calling the static block, no other thread should be able to access it. The bug may be that another thread can acquire a lock on the class before initialisation is finished. :(

I would suggest you structure you code so you don't need such locking on startup. It does sound rather complicated. e.g. in your example Client doesn't need to extends client and the instance could be initialised on the line its declared on. I would consider the following structure.

enum Client implements AppClient {
    INSTANCE;

    public void hello() {
        System.out.println("Hello Client");
    }
}

You can make Client mutable or use delegation so it doesn't expose the fact it can change state (or implementation)

The JLS 12.4.2 explicitly states in (6) that while the initializer executes, locks are released. So i think you see a valid execution path. You may be better of

• killing the static initializer and do synchronized lazy initializing in the accessor
• manually synchronize the static initializer code

    public synchronized static AppClient getInstance() {
      synchronized(Client.class) {
          if (instance == null) {
            instance = new InnerClient();
            doSomethingThatWillCallClientGetInstanceSeveralTimes();
          }
          return instance;
      }
    }

EDIT

Even better - after re-reading this paragraph in the spec, it is even possible to completely remove the synchronization in your original example - the VM will take care of it.

EDIT

Sorry for misguiding - even more detailed reading should reveal that in (2), the second thread can't acquire the lock (after detecting ongoing initialization, it "waits").