RE: [Quickfix-developers] SocketConnection session registration BUG

[Sean K. <Sea...@Pi...>]

I was putting this code in, but there is neither readFromSocket() nor
readMessages() in the SocketConnection object/base in 1.9.4.  Is this
the version in CVS?  I would prefer to patch the released source,
if possible.  The function taken directly from 1.9.4 is as follows:

bool SocketConnection::read( SocketAcceptor& a, SocketServer& s )
{ QF_STACK_PUSH(SocketConnection::read)

  std::string msg;
  try
  {
    if ( !readMessage( msg ) ) return false;

    if ( !m_pSession )
    {
      m_pSession =3D Session::lookupSession( msg, true );
      if ( m_pSession )
        m_pSession =3D Session::registerSession( =
m_pSession->getSessionID() );
      if ( m_pSession )
        m_pSession =3D a.getSession( msg, *this );
    }

    if ( m_pSession )
      m_pSession->next( msg );
    else
      s.getMonitor().drop( m_socket );
    return true;
  }
  catch ( RecvFailed& )
  {
    s.getMonitor().drop( m_socket );
  }
  catch ( InvalidMessage& )
  {
    if ( !m_pSession->isLoggedOn() )
      s.getMonitor().drop( m_socket );
  }
  return true;

  QF_STACK_POP
}

Will modifying the code to be the following (with the extra
isSessionRegistered check) be a sufficient fix?  Or is it
necessary to pull the call to readMessage and Session::next
into the if (!m_pSession) block?=20

bool SocketConnection::read( SocketAcceptor& a, SocketServer& s )
{ QF_STACK_PUSH(SocketConnection::read)

  std::string msg;
  try
  {
    if ( !readMessage( msg ) ) return false;

    if ( !m_pSession )
    {
      m_pSession =3D Session::lookupSession( msg, true );
	if ( m_pSession &&
           Session::isSessionRegistered(m_pSession->getSessionID()) )
        m_pSession =3D 0;
      if ( m_pSession )
        m_pSession =3D a.getSession( msg, *this );
      if ( m_pSession )
        m_pSession =3D Session::registerSession( =
m_pSession->getSessionID() );
    }

    if ( m_pSession )
      m_pSession->next( msg );
    else
      s.getMonitor().drop( m_socket );
    return true;
  }
  catch ( RecvFailed& )
  {
    s.getMonitor().drop( m_socket );
  }
  catch ( InvalidMessage& )
  {
    if ( !m_pSession->isLoggedOn() )
      s.getMonitor().drop( m_socket );
  }
  return true;

  QF_STACK_POP
}

--Sean

> -----Original Message-----
> From: Oren Miller [mailto:or...@qu...]
> Sent: Tuesday, June 28, 2005 9:27 AM
> To: Sean Kirkpatrick; qui...@li...
> Subject: Re: [Quickfix-developers] SocketConnection session=20
> registration
> BUG
>=20
>=20
> Sean,
>=20
> When applying your patch, other acceptance tests break.  In=20
> fact it results=20
> in a crash during a typical multiple logon scenario.  (If you=20
> run your=20
> acceptance tests, it should reveal the same thing to you). =20
> I've included=20
> another implementation of SocketConnection::read which passes all the=20
> acceptance tests and should also fix the bug you've reported.=20
>  Please report=20
> back if that is the case:
>=20
> bool SocketConnection::read( SocketAcceptor& a, SocketServer& s )
> { QF_STACK_PUSH(SocketConnection::read)
>=20
>   std::string msg;
>   try
>   {
>     readFromSocket();
>=20
>     if ( !m_pSession )
>     {
>       if ( !readMessage( msg ) ) return false;
>       m_pSession =3D Session::lookupSession( msg, true );
>       if( m_pSession &&=20
> Session::isSessionRegistered(m_pSession->getSessionID()) )
>         m_pSession =3D 0;
>       if( m_pSession )
>         m_pSession =3D a.getSession( msg, *this );
>       if( m_pSession )
>         m_pSession->next( msg );
>       if( !m_pSession )
>       {
>         s.getMonitor().drop( m_socket );
>         return false;
>       }
>=20
>       Session::registerSession( m_pSession->getSessionID() );
>     }
>=20
>     readMessages( s.getMonitor() );
>     return true;
>   }
>   catch ( SocketRecvFailed& e )
>   {
>     if( m_pSession )
>       m_pSession->getLog()->onEvent( e.what() );
>     s.getMonitor().drop( m_socket );
>   }
>   catch ( InvalidMessage& )
>   {
>     s.getMonitor().drop( m_socket );
>   }
>   return false;
>=20
>   QF_STACK_POP
> }
> ----- Original Message -----=20
> From: "Sean Kirkpatrick" <Sea...@Pi...>
> To: <qui...@li...>
> Sent: Monday, June 27, 2005 10:34 AM
> Subject: [Quickfix-developers] SocketConnection session=20
> registration BUG
>=20
>=20
> QuickFIX Documentation:=20
> http://www.quickfixengine.org/quickfix/doc/html/index.html
> QuickFIX FAQ:=20
http://www.quickfixengine.org/wikifix/index.php?QuickFixFAQ
QuickFIX Support: http://www.quickfixengine.org/services.html

I believe that there is a very simple fix that will correct this issue.  =
The
order of the following code in SocketConnection::read (the LOGDEBUG =
lines=20
are
for the logging I added):

  if ( m_pSession ) {
    LOGDEBUG(funcname,"registering session=20
[%s]",m_pSession->getSessionID().toString().c_str());
    m_pSession =3D Session::registerSession( m_pSession->getSessionID() =
);
  }

  if ( m_pSession ) {
    LOGDEBUG(funcname,"getting SocketAcceptor session =
[%s]",msg.c_str());
    m_pSession =3D a.getSession( msg, *this );
  }

needs to be reversed to be:

  if ( m_pSession ) {
    LOGDEBUG(funcname,"getting SocketAcceptor session =
[%s]",msg.c_str());
    m_pSession =3D a.getSession( msg, *this );
  }

  if ( m_pSession ) {
    LOGDEBUG(funcname,"registering session=20
[%s]",m_pSession->getSessionID().toString().c_str());
    m_pSession =3D Session::registerSession( m_pSession->getSessionID() =
);
  }