Deadlock when closing session in SessionStateListener

[rforrai]

When close is called from SessionStateListener, it can result in deadlock.

I tried it on versions 2.3.7, 2.3.11, 3.0.0 and current master.

Minimal example to reproduce:

SMPPSession session = new SMPPSession(new SynchronizedPDUSender(new DefaultPDUSender(new DefaultComposer())),
                            new DefaultPDUReader(), SocketConnectionFactory.getInstance());
session.setEnquireLinkTimer(2000);
session.connectAndBind("localhost", 8056,
        new BindParameter(BindType.BIND_TRX, null, null, null,
            TypeOfNumber.INTERNATIONAL, NumberingPlanIndicator.UNKNOWN, null), 2000);

session.addSessionStateListener((newState, oldState, source) -> {
    if (SessionState.UNBOUND.equals(newState)) {
        try {
            // Wait until we are sure EnquireLinkSender's wait period (500ms)
            // has passed, and the EnquireLinkSender thread has called getSessionState.
            // getSessionState will wait for lock held by this thread (pool-1-thread-2).
            Thread.sleep(1000);
        } catch (InterruptedException e) {
        }

        // The close tries to EnquireLinkSender.join, but EnquireLinkSender is waiting for the lock
        // held by this thread (pool-1-thread-2).
        // The result is deadlock.
        source.close();
    }
});

// SMPP server should call unbind, so the listener is triggered

Relevant logs from thread dump:

"pool-1-thread-2" prio=0 tid=0x0 nid=0x0 waiting on condition
     java.lang.Thread.State: WAITING
on org.jsmpp.session.AbstractSession$EnquireLinkSender@7ab31eb5
    at java.lang.Object.wait(Native Method)
    at java.lang.Thread.join(Thread.java:1257)
    at java.lang.Thread.join(Thread.java:1331)
    at org.jsmpp.session.AbstractSession.close(AbstractSession.java:269)
    at org.jsmpp.examples.TestClient.lambda$main$0(TestClient.java:36)
    at org.jsmpp.examples.TestClient$$Lambda$3/1896277646.onStateChange(Unknown Source)
    at org.jsmpp.session.AbstractSessionContext.fireStateChanged(AbstractSessionContext.java:87)
    at org.jsmpp.session.SMPPSessionContext.changeState(SMPPSessionContext.java:61)
    at org.jsmpp.session.AbstractSessionContext.unbound(AbstractSessionContext.java:64)
    at org.jsmpp.session.SMPPSession$ResponseHandlerImpl.notifyUnbonded(SMPPSession.java:602)
    at org.jsmpp.session.state.AbstractGenericSMPPSessionBound.processUnbind(AbstractGenericSMPPSessionBound.java:85)
    at org.jsmpp.session.PDUProcessTask.run(PDUProcessTask.java:119)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:750)

"EnquireLinkSender-5b4f5250" prio=0 tid=0x0 nid=0x0 blocked
     java.lang.Thread.State: BLOCKED
on org.jsmpp.session.SMPPSessionContext@6b3fb87d owned by "pool-1-thread-2" Id=18
    at org.jsmpp.session.SMPPSessionContext.getSessionState(SMPPSessionContext.java:39)
    at org.jsmpp.session.AbstractSession.getSessionState(AbstractSession.java:140)
    at org.jsmpp.session.AbstractSession$EnquireLinkSender.run(AbstractSession.java:529)

"PDUReaderWorker-5b4f5250" prio=0 tid=0x0 nid=0x0 blocked
     java.lang.Thread.State: BLOCKED
on org.jsmpp.session.SMPPSessionContext@6b3fb87d owned by "pool-1-thread-2" Id=18
    at org.jsmpp.session.SMPPSessionContext.getSessionState(SMPPSessionContext.java:39)
    at org.jsmpp.session.AbstractSession.getSessionState(AbstractSession.java:140)
    at org.jsmpp.session.SMPPSession$PDUReaderWorker.readPDU(SMPPSession.java:721)
    at org.jsmpp.session.SMPPSession$PDUReaderWorker.run(SMPPSession.java:673)

Is it possible to close the session in SessionStateListener on unbind without getting into deadlock? Or should we close the session in other way when server initiates it?

[bukefalos]

+1

[pmoerenhout]

Thanks, I'll try to reproduce it...

[Archpanda]

If I may ask, why is getSessionState() synchronized in SMPPSessionContext ? It only fetches enum values, whatever the implementation.

[pmoerenhout]

I will test it without the synchronized. Indeed when returning a enum, the synchronized doesn't make sense.

[PascalSchumacher]

I guess this can be closed now that #198 was merged?

[rforrai]

I tested on latest master, and deadlock no longer occurs. This can be closed.