Java - static block and Thread synchronisation issue - a case in hand

Question

What is to be done?

Download logica smpp jar (215 KB) from here: http://opensmpp.logica.com/CommonPart/Download/library_1_3/smpp_full.tar.gz

Write a small test code:

package com.logica.smpp;

import com.logica.smpp.pdu.DataSM;
import com.logica.smpp.pdu.Outbind;

public class PDUTest {
    public static void main(String... args) throws InterruptedException {
        Thread thread1 = new Thread(new Runnable() {
            @Override
            public void run() {
                System.out.println(new DataSM().debugString());
            }
        });
        thread1.setName("ONE");

        Thread thread2 = new Thread(new Runnable() {
            @Override
            public void run() {
                System.out.println(new Outbind().debugString());
            }
        });
        thread2.setName("TWO");

        thread1.start();
        thread2.start();
    }
}

Run this main method.

What happens?

The Threads are blocked (Presumably waiting for each-other)

My analysis:

Both DataSM and Outbind classes have a common ancestor PDU, which has a static block with the following code:

static {
        pduList = new Vector(30,4);
        pduList.add(new BindTransmitter());
        pduList.add(new BindTransmitterResp());
        pduList.add(new BindReceiver());
        pduList.add(new BindReceiverResp());
        pduList.add(new BindTransciever());
        pduList.add(new BindTranscieverResp());
        pduList.add(new Unbind());
        pduList.add(new UnbindResp());
        pduList.add(new Outbind());
        pduList.add(new SubmitSM());
        pduList.add(new SubmitSMResp());
        pduList.add(new SubmitMultiSM());
        pduList.add(new SubmitMultiSMResp());
        pduList.add(new DeliverSM());
        pduList.add(new DeliverSMResp());
        pduList.add(new DataSM());
        pduList.add(new DataSMResp());
        pduList.add(new QuerySM());
        pduList.add(new QuerySMResp());
        pduList.add(new CancelSM());
        pduList.add(new CancelSMResp());
        pduList.add(new ReplaceSM());
        pduList.add(new ReplaceSMResp());
        pduList.add(new EnquireLink());
        pduList.add(new EnquireLinkResp());
        pduList.add(new AlertNotification());
        pduList.add(new GenericNack());
    }

It creates pduList so that it can create objects of its children like BindTransmitter, DataSM, Outbind etc via a factory method which it provides at createPDU

So when my test application gets executed, ONE Thread enters PDU's static method (while initialising DataSM). And the TWO Thread, which has started initialising Outbind waits for ONE to finish initialising PDU.
But at some point in ONE which is running static method of PDU, it tries to initialise Outbind, and seeing that TWO has already started the same thing, it waits for TWO to finish.
So ONE and TWO are waiting for each-other to finish
How can i be confident that this issue is related to static block loading?
Adding just this following one line as the first statement in the main method of test code, makes it work and the Threads do not block anymore:
```
Class.forName("com.logica.smpp.pdu.PDU");
```

My Questions are these:

Is my analysis correct?
Is this a known Thread synchronisation issue concerning with static blocks?
Any thumb rule that needs to be practiced to not stumble onto this situation?

Update

Adding the factory method for PDU here:

public static final PDU createPDU(int commandId)
{
    int size = pduList.size();
    PDU pdu = null;
    PDU newInstance = null;
    for (int i = 0; i < size; i++) {
        pdu = (PDU)pduList.get(i);
        if (pdu != null) {
            if (pdu.getCommandId() == commandId) {
                try {
                    newInstance = (PDU)(pdu.getClass().newInstance());
                } catch (IllegalAccessException e) {
                } catch (InstantiationException e) {
                }
                return newInstance;
            }
        }
    }
    return null;
}

What do the constructors of DataSM, Outbind and other child classes of PDU do?
Nothing, except initialising a few instance variables. These are POJOs. They do NOT hold on to any external resources like file, database etc.

Can you post the code that are actually synchronized and are executed by your code? — Bhesh Gurung, Feb 24 '13 at 05:44
There is no other code, just the smpp library and my test code which i have given above. Run the test code and you'll see the results i'm talking about. — imdahmd, Feb 24 '13 at 05:48
`via a factory method which it provides at createPDU` - paste this method so we can see it. — xagyg, Feb 24 '13 at 13:16
what do the constructors for `Outbind` and `DataSM` look like? — xagyg, Feb 24 '13 at 13:17

score 1 · Answer 1 · edited May 23 '17 at 12:18

1

Your threads may be blocking, but not for the reason you think. Static initializers are executed when the class is loaded, not when an instance is created. So, you don't have your two objects 'entering' the static initializer and at some point, becoming deadlocked on some shared variable.

Without knowing the exact details of the library you are using, its hard to diagnose what the real issue may be, but I would recommend dumping your threads and analyzing them with a decent tool.

edited May 23 '17 at 12:18

Community

1
1

answered Feb 24 '13 at 05:35

Perception

79,279
19
185
195

But as you may notice from my test application, both the both the classes of `DataSM` and `Outbind` are being loaded for the first time when the `Thread`s start executing. So the static block of their ancestor `PDU` is being entered in these `Thread`'s scope. Also the source for this library is available. These objects are POJOs. They don't hold on to any external resources at all. – imdahmd Feb 24 '13 at 05:38
2

Unless your classes are being loaded in different class loaders, this simply cannot be the case. Static initializers are thread safe, and are run once per classloader. – Perception Feb 24 '13 at 05:41
@imdhmd - No, they're not. Static initializers are actually inherently thread safe. It's one way to implement a singleton in Java, as a matter of fact (using the static initializer to create the instance of the object to ensure only one instance is ever created). – Brian Roach Feb 24 '13 at 05:42
@Brain I would love for you to try what i'm saying here in my question. I think you might be surprised. – imdahmd Feb 24 '13 at 05:46
In my `main` method, i added this as first statement: `Class.forName("com.logica.smpp.pdu.PDU");` And then it works like charm. No Thread blocks. Everything goes smoothly. – imdahmd Feb 24 '13 at 05:56

score 1 · Answer 2 · answered Feb 24 '13 at 20:23

If one class's initializer depends on another class, and vice versa, it can cause deadlock if two classes are initialized in 2 different threads. You are very likely correct in your analysis.

If these two classes are initialized in the same thread, there won't be deadlock; nevertheless, circular dependency should be avoided. In this example, the pduList stuff should be in its own class.

score 0 · Answer 3 · answered Feb 24 '13 at 05:40

You can use the jstack tool to print out thread stack:

jstack <pid of your java process>

You can find out pid via jps command. For example, the jps command output of my machine:

eric@erics-MacBook-Pro:~$ jps
577 Jps
448

Then the jstack command with the pid 448:

jstack 448

And part of its output:

"Worker-46" prio=5 tid=10f543800 nid=0x11811e000 in Object.wait() [11811d000]
java.lang.Thread.State: TIMED_WAITING (on object monitor)
at java.lang.Object.wait(Native Method)
- waiting on <7a0001a80> (a org.eclipse.core.internal.jobs.WorkerPool)
at org.eclipse.core.internal.jobs.WorkerPool.sleep(WorkerPool.java:188)
- locked <7a0001a80> (a org.eclipse.core.internal.jobs.WorkerPool)
at org.eclipse.core.internal.jobs.WorkerPool.startJob(WorkerPool.java:220)
at org.eclipse.core.internal.jobs.Worker.run(Worker.java:50)

You can see "Worker-46" is blocked in Object.wait().

It should be clear on why the code blocking when you get your thread stack.

score 0 · Answer 4 · answered Feb 25 '13 at 00:22

0

Please add lock(pduList) as FIRST statement in createPDU. Close the lock block immediately after the return. Try that please. Let me know how you go with it.

e.g.

public static final PDU createPDU(int commandId)
{
  lock(pduList) {
    int size = pduList.size();
    PDU pdu = null;
    PDU newInstance = null;
    for (int i = 0; i < size; i++) {
        pdu = (PDU)pduList.get(i);
        if (pdu != null) {
            if (pdu.getCommandId() == commandId) {
                try {
                    newInstance = (PDU)(pdu.getClass().newInstance());
                } catch (IllegalAccessException e) {
                } catch (InstantiationException e) {
                }
                return newInstance;
            }
        }
    }
    return null;
  } // end lock  
}

answered Feb 25 '13 at 00:22

xagyg

9,562
2
32
29

that doesnt help @xagyg, as you can notice, at NO point in my test code do i invoke the createPDU method. – imdahmd Feb 25 '13 at 01:08
Did you try it? Are you sure it doesn't get called at all? Just because you didn't call it doesn't mean it doesn't get called. Just saying ... are you sure? – xagyg Feb 25 '13 at 01:15
Also, don't gobble up exceptions (like you are doing in `createPDU`). Make `createPDU` throw exception and remove those catches that hide any exceptions. – xagyg Feb 25 '13 at 01:18
yes, i'm sure it doesn't get called at all. And those exceptions do not require handling, since the pduList was initialised in the static block with classes of type PDU. – imdahmd Feb 25 '13 at 01:32
In any case, I would at least put `e.printStackTrace()` inside each of those catch blocks. – xagyg Feb 25 '13 at 03:19