jain-sip
jain-sip copied to clipboard
NioPipelineParser: possible message ordering issue in Dispatch?
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 Yes this issue exists! Did you get any update/fix for this?
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 thats great to know that it is working. Is your product still using this stack and have you done any further improvements?