atom feed4 messages in net.java.dev.jain-sip.usersSipServerTransaction and MessageChann...
FromSent OnAttachments
Frederic TardifAug 16, 2012 12:06 pm 
M. RanganathanAug 16, 2012 1:33 pm 
Jean DeruelleAug 17, 2012 1:37 am 
M. RanganathanAug 17, 2012 6:58 am 
Subject:SipServerTransaction and MessageChannel interaction?
From:Frederic Tardif (fred@gmail.com)
Date:Aug 16, 2012 12:06:55 pm
List:net.java.dev.jain-sip.users

Bonjour,

As shown below, I found out the issue I was chasing (JSIP-339) regarding wrong traces logging when sending retransmitted responses though a SipServerTransaction using an unreliable transport (UdpMessageChannel).

I had quite an headache to weave my way through the implementation of the SipTransaction's and I would appreciate if someone could enlighten me with the design intended behind. Here are a couple of caveat I found out:

- A SipTransaction owns an encapsulated MessageChannel. - The MessageChannel can either be associated on construction or set later on. - The MessageChannel kept as a member in a TX only makes sense for reliable transport !!! - If the MessageChannel is UDP, the interior of the UdpMessageChannel will change as further unrelated requests goes. Consequently, the encapsulated messageChannel member will not reflect anymore the original connection used to define the transaction. - This means all public SipTransaction methods (and even internal usage) of messageChannel are bug prone when dealing with a udp transport. - Moreover, why a SipTransaction extends MessageChannel (A extends B but also owns a B.... )??? ObjectOriented-wise, it smells bad... - This is especially error prone since a SipTransaction must expose methods like getPeerAddress() from MessageChannel. It actually implements it by doing encapsulatedMessageChannel.getPeerAddress(). But as stated above, using the encapsulated messageChannel like this will only work if the underlying transport is reliable, otherwise the result will be random according to concurrent udp traffic!!!! - For the present bug, it was only a logging issue, but when checking at all the other classes coupled with a SipTransaction using either getMessageChannel() or any other inherited MessageChannel related method, I am really worried about other ripple effects.

Unless I am missing something, there is an ObjectOriented design inconsistency in the current implementation. I don't know exactly what is the best strategy to address this but I was finding important to raise the flag.

Frederic

---------- Forwarded message ---------- From: <ftar@java.net> Date: Thu, Aug 16, 2012 at 2:11 PM Subject: [jsip~svn:2145] fix for issue : http://java.net/jira/browse/JSIP-339 To: cv@jsip.java.net

Project: jsip Repository: svn Revision: 2145 Author: ftardif Date: 2012-08-16 18:11:35 UTC Link:

Log Message:

------------ fix for issue : http://java.net/jira/browse/JSIP-339 The code was not using the right messageChannel when using unreliable transport causing to log retransmited responses using peerAddress and peerPort of an already reused UdpMessageChannel.

Revisions:

---------- 2145

Modified Paths:

--------------- trunk/src/gov/nist/javax/sip/stack/SIPServerTransaction.java

Diffs:

------ Index: trunk/src/gov/nist/javax/sip/stack/SIPServerTransaction.java =================================================================== --- trunk/src/gov/nist/javax/sip/stack/SIPServerTransaction.java (revision 2144) +++ trunk/src/gov/nist/javax/sip/stack/SIPServerTransaction.java (revision 2145) @@ -1249,17 +1249,12 @@ try { SIPResponse lastReparsedResponse = (SIPResponse) sipStack.getMessageParserFactory().createMessageParser(sipStack).parseSIPMessage(lastResponseAsBytes, true, false, null);

- lastReparsedResponse.setRemoteAddress( - this.getPeerInetAddress()); - lastReparsedResponse.setRemotePort - (this.getPeerPort()); - lastReparsedResponse.setLocalPort( - getMessageChannel().getPort()); - lastReparsedResponse.setLocalAddress( - getMessageChannel() - .getMessageProcessor().getIpAddress()); + lastReparsedResponse.setRemoteAddress(messageChannel.getPeerInetAddress()); + lastReparsedResponse.setRemotePort(messageChannel.getPeerPort()); + lastReparsedResponse.setLocalPort(messageChannel.getPort()); + lastReparsedResponse.setLocalAddress(messageChannel.getMessageProcessor().getIpAddress());

- getMessageChannel().logMessage(lastReparsedResponse, this.getPeerInetAddress(), this.getPeerPort(), System.currentTimeMillis()); + messageChannel.logMessage(lastReparsedResponse, messageChannel.getPeerInetAddress(), messageChannel.getPeerPort(), System.currentTimeMillis()); } catch (ParseException e) { if (logger.isLoggingEnabled(LogWriter.TRACE_DEBUG)) { logger.logDebug("couldn't reparse last response " + new String(lastResponseAsBytes));