Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[master branch] EJBCLIENT-81 Fix potential class initialization deadlock #45

Merged
merged 2 commits into from May 14, 2013
Merged

[master branch] EJBCLIENT-81 Fix potential class initialization deadlock #45

merged 2 commits into from May 14, 2013

Conversation

jaikiran
Copy link
Contributor

@jaikiran jaikiran commented May 9, 2013

https://issues.jboss.org/browse/EJBCLIENT-81 exposes a bug where the static initializer of EJBClientContext would trigger operations like EJB receiver creation and registration against EJBClientContext(s) from within the static initializer. Some of these operations involve acquiring locks for synchronized blocks. What's causing this issue is:

  1. The JVM as per the spec (1http://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.4.2) holds a static initializer lock (L1) for EJBClientContext class instance for thread T1
  2. The static initializer then triggers the ConfigBasedEJBClientContextSelector to setup a EJB receiver context with receivers
  3. Previous step causes the server to send down a cluster topology to the client which then triggered a registration of EJB receiver within the EJB client context in a separate thread T2 (cluster node receiver registrations are automatic and are handled in a background thread).
  4. In the meantime, T1 continues to registers receiver(s) configured in the jboss-ejb-client.properties
  5. So T1 and T2 are simultaneously are registering the receiver(s) to the EJBClientContext instance
  6. The registration processes looks something like:
    1. private static Logger logger = Logger.getLogger(EJBClientContext.class)
    ..
    2. registerEJBReceiver(receiver) {
    3.    synchronized(instanceVariableFoo) {
            ... // do something
    4.      logger.debug("foo bar");
        }

        // do something else

    5.   synchronized(instanceVariableFoo) {
            .. // some more work
        }

    }
    ...

It so happens that T1 waits for a lock L2 on "instanceVariableFoo" on line 5. The lock L2 is held by the other thread T2 between lines 2 through 4. So T1 waits. Now T2 reaches the logger statement on line 4. The logger requires the class instance of EJBClientContext which hasn't yet fully initialized (remember all this is triggered via static initializer in that class). Since the class isn't fully initialized the thread T2 tries to get the initialization lock L1 but that's held by T1 and won't release it until the class initialization completes and that won't happen until T2 releases the lock L2, for T1 to finish. This effectively leads to a deadlock which is what is happening in EJBCLIENT-81.

The commit here fixes the issue by _not_ using the static initializer block of EJBClientContext to trigger receiver registration(s) and effectively spawing other threads to register the cluster nodes. Instead it just "constructs" the ConfigBasedEJBClientContextSelector. The receiver registration is done lazily, later on, by the ConfigBasedEJBClientContextSelector

@jaikiran jaikiran merged commit 8a34304 into wildfly:master May 14, 2013
@jaikiran
Copy link
Contributor Author

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant