Re: [Quickfix-developers] SocketConnection session registration BUG

[Oren M. <or...@qu...>]

Yeah, I think that will do although clearly I have not tested that  
exact code block.  The point of the patch is that the session will  
only get registered when all other tests have succeeded.  Just make  
sure that in addition to fixing your problem, you are also still  
passing all the unit/acceptance tests.

--oren

On Jun 28, 2005, at 11:48 AM, Sean Kirkpatrick wrote:

> 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 = Session::lookupSession( msg, true );
>       if ( m_pSession )
>         m_pSession = Session::registerSession( m_pSession- 
> >getSessionID() );
>       if ( m_pSession )
>         m_pSession = 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?
>
> 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 = Session::lookupSession( msg, true );
>     if ( m_pSession &&
>            Session::isSessionRegistered(m_pSession->getSessionID()) )
>         m_pSession = 0;
>       if ( m_pSession )
>         m_pSession = a.getSession( msg, *this );
>       if ( m_pSession )
>         m_pSession = 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
>> registration
>> BUG
>>
>>
>> Sean,
>>
>> When applying your patch, other acceptance tests break.  In
>> fact it results
>> in a crash during a typical multiple logon scenario.  (If you
>> run your
>> acceptance tests, it should reveal the same thing to you).
>> I've included
>> another implementation of SocketConnection::read which passes all the
>> acceptance tests and should also fix the bug you've reported.
>>  Please report
>> back if that is the case:
>>
>> bool SocketConnection::read( SocketAcceptor& a, SocketServer& s )
>> { QF_STACK_PUSH(SocketConnection::read)
>>
>>   std::string msg;
>>   try
>>   {
>>     readFromSocket();
>>
>>     if ( !m_pSession )
>>     {
>>       if ( !readMessage( msg ) ) return false;
>>       m_pSession = Session::lookupSession( msg, true );
>>       if( m_pSession &&
>> Session::isSessionRegistered(m_pSession->getSessionID()) )
>>         m_pSession = 0;
>>       if( m_pSession )
>>         m_pSession = a.getSession( msg, *this );
>>       if( m_pSession )
>>         m_pSession->next( msg );
>>       if( !m_pSession )
>>       {
>>         s.getMonitor().drop( m_socket );
>>         return false;
>>       }
>>
>>       Session::registerSession( m_pSession->getSessionID() );
>>     }
>>
>>     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;
>>
>>   QF_STACK_POP
>> }
>> ----- Original Message -----
>> From: "Sean Kirkpatrick" <Sean.Kirkpatrick@PipelineTrading.com>
>> To: <qui...@li...>
>> Sent: Monday, June 27, 2005 10:34 AM
>> Subject: [Quickfix-developers] SocketConnection session
>> registration BUG
>>
>>
>> QuickFIX Documentation:
>> http://www.quickfixengine.org/quickfix/doc/html/index.html
>> QuickFIX FAQ:
>>
> 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
> are
> for the logging I added):
>
>   if ( m_pSession ) {
>     LOGDEBUG(funcname,"registering session
> [%s]",m_pSession->getSessionID().toString().c_str());
>     m_pSession = Session::registerSession( m_pSession->getSessionID 
> () );
>   }
>
>   if ( m_pSession ) {
>     LOGDEBUG(funcname,"getting SocketAcceptor session [% 
> s]",msg.c_str());
>     m_pSession = a.getSession( msg, *this );
>   }
>
> needs to be reversed to be:
>
>   if ( m_pSession ) {
>     LOGDEBUG(funcname,"getting SocketAcceptor session [% 
> s]",msg.c_str());
>     m_pSession = a.getSession( msg, *this );
>   }
>
>   if ( m_pSession ) {
>     LOGDEBUG(funcname,"registering session
> [%s]",m_pSession->getSessionID().toString().c_str());
>     m_pSession = Session::registerSession( m_pSession->getSessionID 
> () );
>   }
>
>
>
>