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

[Dario Bozzali]

Hi Scott,
Thank you for your reply. At the moment I don't have logs that we need, but I'm still trying to analyze/reproduce the issue. I was wondering if it could be something related to multi-threaded stack support, since if it is enabled (as in my scenario when fault occurred) Dns, TransactionController and TransportSelector have their own running threads. What's your opinion about that?
Thank you and best regards,
Dario

From: slgodin at gmail.com [mailto:slgodin at gmail.com] On Behalf Of Scott Godin
Sent: giovedì 18 febbraio 2016 20.24
To: Dario Bozzali <Dario.Bozzali at ifmgroup.it>
Cc: resiprocate-devel at resiprocate.org
Subject: Re: [reSIProcate] Question about TransactionState::handleSync() method

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<mailto: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<mailto: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/20160219/d5471afd/attachment.htm>