[Quickfix-developers] ThreadedSocketInitiator sync problems in 1.1.1

[Gene G. <mus...@ya...>]

I wrote a pair of buyer/exchange apps borrowing
liberally from the sample code and using quickfix
threaded acceptor/initiator classes. Buyer app
initiates connection and in the App::OnRun generates
SingleOrderRequests in a tight loop. Once a sufficient
number of requests have been generated onRun exits.

I have run into a few sync problem scenarios in the
buyer app:
Scenario 1:
once onRun returns, ThreadedSocketInitiator returns
control from OnStart to main app, which tries to clean
up, Virtual destructor chain is called, getting to
~Initiator. Initiator cleans up all the session
pointers, with guarding mutex wide open because while
ThreadedSocketInitiator is derived from Initiator,
both of them have their separate private m_mutex.

Meanwhile the connection queue and SocketThread both
need pointer to Initiator and its sessions on other
threads. readQueue thread makes a call Session->next
and calling a method of a deleted pointer leads to sad
consequences.

The above problem will probably go away once
ThreadedSocketInitiator uses Initiator's m_mutex
(which will have to be "protected" instead of
"private") or guards its own destructor with its own
m_mutex.


Another independent scenario which manifested itself
once I added code to post a logout message in my
buyer's onRun before exiting.


Two threads of the Threadedinitiator are  deadlocked.
One sits in OnStop. It has grabbed the mutex, and is
waiting to join the SocketThread. The SocketThread
however is also trying to get out. It wants to
unregister by calling removeThread, but the mutex has
been grabbed by onStop which is waiting for
SocketThread to return...
The problem is complicated because SocketThread is
attempting to close its own handle (by the way,
pthread_detach has to be called on pthread systems,
similarly to CloseHandle), which of course makes it
impossible to join from elsewhere(for instance from
OnStop).  The solution here is to not close handle and
erase the pair from the threads map during shutdown
processing (when m_stop is raised), and instead
relegate this to the thread that performes the join.

Here it would be helpful to use a robust Thread class,
which would make sure that handle is cleaned up both
after join and in destruction. Although not without a
fault,a decent portable implementation of such class
is  in the Boost Thread library
(http://www.boost.org/libs/thread/doc/index.html) That
implementation also includes a thread_group class
which is quite similar in functionality to
SocketToThread map with robust remove/add/join_all
methods.

The third and related to second problem is that m_stop
is not guarded between
ThreadedSocketInitiator::SocketThread and OnStop
methods.


Fourth problem is that at least in one place
(ThreadedSocketInitiator::removeThread), and perhaps
more, the mutex is locked explicitly, instead of via
Locker. If STL throws an exception (however unlikely,
it may happen!) between explicit lock/unlocks, mutex
will stay locked, with dire consequences. Explicit
locking/unlocking of mutex should carry a pretty big
comment next to it, as it is very easy to slip an
exception producing code between them, and is best
reserved for complicated syncing, as opposed to plain
guarding.

If finer grained then the method scope locking is
required, it can usually be achieved using an
anonymous C++ block
c::f() 
{
unprotected code;
{
locker l(mutex);
protected code;
}
unprotected code;
}

Gene Gorokhovsky
 

__________________________________________________
Do You Yahoo!?
Yahoo! Health - Feel better, live better
http://health.yahoo.com