Re: [reSIProcate-users] Unexpected TCP connection termination disturbing call setup

["Julio Cesar Esteves Cabezas" <jcabezas@xxxxxxxxxxxxx>]

Hi Scott,

  The new code for the the fuctions worked fine !
  As far as I could test, the TCP connections did not drop and SIP messages
were exchanged correctly.
  
Many thanks,
Julio.

----------------------------------------------------------------------------
-
From: slgodin@xxxxxxxxx [mailto:slgodin@xxxxxxxxx] On Behalf Of Scott Godin
Sent: quarta-feira, 16 de outubro de 2013 13:14
To: Julio Cabezas
Cc: resiprocate-users@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate-users] Unexpected TCP connection termination
disturbing call setup

Ok - I can't see what would reset the WSAGetLastError - but clearly we
shouldn't be relying on it not being reset that many calls after the actual
write call.  I'm thinking I will instead rewrite the logic so that the
EAGAIN and EWOULDBLOCK handling is in the TcpConnection code itself.  I
checked the TlsConnection class and it handles these similar conditions
within the subclass.

Here is a copy of the two new functions for you to try out:
int 
TcpConnection::write( const char* buf, const int count )
{
   //DebugLog (<< "Writing " << buf);   // Note:  this can end up writing
garbage to the logs following the message for non-null terminated buffers

   assert(buf);
   assert(count > 0);

#if defined(WIN32)
   int bytesWritten = ::send(getSocket(), buf, count, 0);
#else
   int bytesWritten = ::write(getSocket(), buf, count);
#endif

   if (bytesWritten == INVALID_SOCKET)
   {
      int e = getErrno();
      //setFailureReason(TransportFailure::ConnectionException, e+1000);
      if (e == EAGAIN || e == EWOULDBLOCK) // Treat EGAIN and EWOULDBLOCK as
the same:
http://stackoverflow.com/questions/7003234/which-systems-define-eagain-and-e
wouldblock-as-different-values
      {
          // TCP buffers are backed up - we couldn't write anything - but we
shouldn't treat this an error - return we wrote 0 bytes
          return 0;
      }
      InfoLog (<< "Failed write on " << getSocket() << " " << strerror(e));
      Transport::error(e);
      return -1;
   }
   
   return bytesWritten;
}

int
Connection::performWrite()
{
....
   int nBytes = write(data.data() + mSendPos,int(data.size() - mSendPos));

   //DebugLog (<< "Tried to send " << data.size() - mSendPos << " bytes,
sent " << nBytes << " bytes");

   if (nBytes < 0)
   {
      //fail(data.transactionId);
      InfoLog(<< "Write failed on socket: " << this->getSocket() << ",
closing connection");
      return -1;
   }
   else if (nBytes == 0)
   {
       // Nothing was written - likely socket buffers are backed up and
EWOULDBLOCK was returned
       // no need to do calculations in else statement
       return 0;
   }
   else
   {
      // Safe because of the conditional above ( < 0 ).
      Data::size_type bytesWritten = static_cast<Data::size_type>(nBytes);
      mSendPos += bytesWritten;
      if (mSendPos == data.size())
      {
         mSendPos = 0;
         removeFrontOutstandingSend();
      }
      return bytesWritten;
   }
}

Scott

On Wed, Oct 16, 2013 at 11:30 AM, Julio Cesar Esteves Cabezas
<jcabezas@xxxxxxxxxxxxx> wrote:
Hello,

  Using getErrno() instead of errno in Connection.cxx as suggested did NOT
work.

  I added then some additional calls to getErrno() inside TcpConnection.cxx
after the original

int e = getErrno();

  and could see that after the call

InfoLog (<< "Failed write on " << getSocket() << " " << strerror(e));
  getErrno() returns 0.

  MSDN doc for WSAGetLastError()
(http://msdn.microsoft.com/en-us/library/windows/desktop/ms741580%28v=vs.85%
29.aspx)
advises:
"If a function call's return value indicates that error or other relevant
data was returned in the error code,
WSAGetLastError should be called immediately. This is necessary because some
functions may reset the last extended
error code to 0 if they succeed, overwriting the extended error code
returned by a previously failed function. "

  That reset seems to be happening inside InfoLog() or getSocket() or
strerror().

Regards,
Julio.

----------------------------------------------------------------------------
----------------------------------------------------------

From: slgodin@xxxxxxxxx [mailto:slgodin@xxxxxxxxx] On Behalf Of Scott Godin
Sent: terça-feira, 15 de outubro de 2013 18:28
To: Julio Cabezas
Cc: resiprocate-users@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate-users] Unexpected TCP connection termination
disturbing call setup

Hi Julio,

I think I see the problem - the code in TcpTransport uses getErrno() helper
API (which calls WSAGetLastError instead of using the global errno that *nix
systems use) and the code in conneciton just uses errno.  Can you please try
just changing the following lines in Connection.cxx (near line 168) to use
the getErrno() call instead?

If this works for you I will commit this fix.  Thanks for your patience and
help in tracking this down.

   int nBytes = write(data.data() + mSendPos,int(data.size() - mSendPos));
   if (nBytes < 0)
   {  
      if (getErrno() != EAGAIN && getErrno() != EWOULDBLOCK) // Treat EGAIN
and EWOULDBLOCK as the
same: http://stackoverflow.com/questions/7003234/which-systems-define-eagain
-and-ewouldblock-as-different-values
      {
         //fail(data.transactionId);
         InfoLog(<< "Write failed on socket: " << this->getSocket() << ",
closing connection");
         return -1;
      }
      else
      {
         return 0;
      }
   }

Scott


_______________________________________________
resiprocate-users mailing list
resiprocate-users@xxxxxxxxxxxxxxx
List Archive: http://list.resiprocate.org/archive/resiprocate-users/