[reSIProcate] Question about TransactionState::handleSync() method

[Scott Godin]

I believe mNextTransmission is always expected to be valid during the send
process.  I think we need to understand how we got a DNS result and
mNextTransmission is not set.  Do you happen to have STACK or DEBUG level
logs of this occurring?

Thanks,
Scott

On Wed, Feb 17, 2016 at 3:53 AM, Dario Bozzali <Dario.Bozzali at ifmgroup.it>
wrote:

> Hi all,
>
> During a test with one of the lastest revision of reSIProcate library (Revision: b8111abeb6672cb00a42d45dfa734f8edfb04e99, Date: 09/11/2015) I encountered a strange crash in my application in TransactionState::handleSync() method.
>
> Crash occurred since mNextTransmission was equal to NULL while trying to assign value to mTarget.mTransportKey (please see excerpt of code reported below).
>
> It doesn't happen always. It occurred once and I'm still not able to reproduce always the issue, but I decided to write to you hoping that someone could help me to analyze this problem.
>
> In my opinion crash occurred on a Client transaction on receiving a retransmitted 200 OK, so on retransmitting ACK message.
>
> My doubt is the following one: Are we sure that mNextTransmission should be valid in handleSync()? In sendCurrentToWire() method we check if mNextTransmission is valid, so I was wondering if it should be better to check mNextTransmission also in handleSync() method.
>
> What is your opinion regarding this?
>
> Thank you in advance and best regards,
>
> Dario
>
>
>
>
>
> void
>
> TransactionState::handleSync(DnsResult* result)  // !slg! it is strange that we pass the result in here, then more or less ignore it in the method
>
> {
>
>    StackLog (<< *this << " got DNS result: " << *result);
>
>
>
>    // .bwc. Were we expecting something from mDnsResult?
>
>    if (mPendingOperation == Dns)
>
>    {
>
>       resip_assert(mDnsResult);
>
>       switch (mDnsResult->available())
>
>       {
>
>          case DnsResult::Available:
>
>             mPendingOperation=None;
>
>             mTarget = mDnsResult->next();
>
>             // below allows TU to know which transport we send on
>
>
> // (The Via mechanism for setting transport doesn't work for TLS)
>
>
>             mTarget.mTransportKey = mNextTransmission->getDestination().mTransportKey;
>
>             processReliability(mTarget.getType());
>
>             sendCurrentToWire();
>
>             break;
>
> ...
>
> }
>
>
>
> void
>
> TransactionState::sendCurrentToWire()
>
> {
>
>    if(!mMsgToRetransmit.empty())
>
>    {
>
>       if(mController.mStack.statisticsManagerEnabled())
>
>       {
>
>          mController.mStatsManager.retransmitted(mCurrentMethodType,
>
>                                                    isClient(),
>
>                                                    mCurrentResponseCode);
>
>       }
>
>        mController.mTransportSelector.retransmit(mMsgToRetransmit);
>
>    }
>
>    else if(mNextTransmission) // initial transmission; need to determine target
>
>    {
>
>       SipMessage* sip=mNextTransmission;
>
>       TransportSelector::TransmitState transmitState = TransportSelector::Unsent;
>
> ...
>
>       // !bwc! If we don't have DNS results yet, or TransportSelector::transmit
>
>       // fails, we hang on to the full original SipMessage, in the hope that
>
>       // next time it works.
>
>       if (transmitState == TransportSelector::Sent)
>
>       {
>
>          onSendSuccess();
>
>       }
>
>    }
>
>    else
>
>    {
>
>       resip_assert(0);
>
>    }
>
> }
>
>
>
> TransactionState::onSendSuccess()
>
> {
>
> ...
>
>    // !bwc! If mNextTransmission is a non-ACK request, we need to save the
>
>    // initial request in case we need to send a simulated 408 or a 503 to
>
>    // the TU (at least, until we get a response back)
>
>    if(!mNextTransmission->isRequest() || mNextTransmission->method()==ACK)
>
>    {
>
>       delete mNextTransmission;
>
>       mNextTransmission=0;
>
>    }
>
> }
>
>
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20160218/e1addd69/attachment.htm>