jain-sip icon indicating copy to clipboard operation
jain-sip copied to clipboard

NioPipelineParser: possible message ordering issue in Dispatch?

Open TristanPerry opened this issue 6 years ago • 3 comments

Issue:

From looking at NioPipelineParser, I'm wondering whether messages for a specific callId could be processed out of order?

Reason:

#59 / 6e5a46f reworked the sipStack's selfRoutingThreadpoolExecutor to use a ThreadAffinityExecutor which will re-use the same thread executor (from the pool) if the ThreadAffinityIdentifier.getThreadHash() for a submitted object is not null.

#44 / 17743fe reworked the NioPipelineParser to remove the semaphore+queue based solution (for message ordering) with the aim to instead use:

A simple thread pool with thread affinity by call id

I might be wrong, although I don't think this is actually happening within the pipeline? I say this because Dispatch does not implement ThreadAffinityIdentifier and thus ThreadAffinityExecutor.calculateAffinityThread() will just retrieve the next thread (instead of scheduling with the same executor)?

Potential fix:

Change the Dispatch method to be:

public class Dispatch implements Runnable, QueuedMessageDispatchBase, ThreadAffinityIdentifier {
    	String callId;
        UnparsedMessage unparsedMessage;
    	long time;

    	public Dispatch(UnparsedMessage unparsedMsg, String callId) {
    		this.unparsedMessage = unparsedMsg;
    		this.callId = callId;
    		time = System.currentTimeMillis();
    	}

        @Override
        public Object getThreadHash() {
            return this.callId;
        }

        ...

TristanPerry avatar Mar 19 '19 15:03 TristanPerry

@TristanPerry Yes this issue exists! Did you get any update/fix for this?

dinoopp avatar Sep 29 '21 04:09 dinoopp

Unfortunately I didn't get any update @dinoopp , although the potential fix that I mention (i.e. ensuring that callId is saved in the class, then overriding the getThreadHash() method) seemed to work fine for us.

TristanPerry avatar Sep 29 '21 09:09 TristanPerry

@TristanPerry thats great to know that it is working. Is your product still using this stack and have you done any further improvements?

dinoopp avatar Sep 29 '21 13:09 dinoopp