< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index  

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


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@xxxxxxxxx [mailto:slgodin@xxxxxxxxx] On Behalf Of Scott Godin
Sent: giovedì 18 febbraio 2016 20.24
To: Dario Bozzali <Dario.Bozzali@xxxxxxxxxxx>
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
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@xxxxxxxxxxx> 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(DnsResultresult)  // !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@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel