How to avoid recursive_mutex

640 views Asked by At

I have a case of recursive_mutex which I'm trying to solve. Here is the piece of code which explains the problem.

 void OnConnectionDisconnected()
    {
        boost::lock_guard<boost::mutex> lock ( m_mutexConnectionSet );      
        IPCSyncConnectionSharedPtrSet::iterator it =      m_IPCSyncConnectionSet.find(spConnection);
        if ( it != m_IPCSyncConnectionSet.end())
        {   
            m_IPCSyncConnectionSet.erase(*it);      
        }
    }

    void ShutdownServer()
    {
        boost::lock_guard<boost::mutex> lock ( m_mutexConnectionSet );      
        IPCSyncConnectionSharedPtrSet::iterator it = m_IPCSyncConnectionSet.begin();    
        for (; it != m_IPCSyncConnectionSet.end(); )
        {
            if (*it) 
            {
                IPCSyncConnectionSharedPtr spIPCConnection = (*it);
                it++;

                //This call indirectly calls OnConnectionDisconnected and erase the connection.
                spIPCConnection->Disconnect();
            }
            else 
            {
                ++it;
            }
        }
    }

OnConnectionDisconnected is invoked on multiple threads (n), as well ShutdownServer is invoked on only one thread at any time when connection is active or being disconnected. ShutdownServer iterates through all connections and calls Disconnect on each one which indirectly calls OnConnectionDisconnected where I actually erase the connection. I have locked mutex before accessing m_IPCSyncConnectionSet since the connectionset gets modified in other threads.

I'll need to use recursive_mutex in above sample code since mutex is locked twice on same thread, when Shutdown is invoked.

Can anyone suggest how can I solve the above problem and avoid recursive_lock ? recurive_mutex will kill you as per this article http://www.fieryrobot.com/blog/2008/10/14/recursive-locks-will-kill-you/

thanks,

1

There are 1 answers

1
Igor R. On BEST ANSWER

In ShutdownServer() work with a temporary copy of the container. Besides the locking issue, it's not a good idea to iterate a container being modified by another function (even if this particular container type doesn't invalidate all the iterators when erasing an element, such a code would be quite fragile and unnecessarily complicated).

void ShutdownServer()
{
    boost::lock_guard<boost::mutex> lock ( m_mutexConnectionSet );
    auto connections = m_IPCSyncConnectionSet;
    lock.unlock();
    for (auto it = connections.begin(); it != connections.end(); ++it)
      if (*it) 
        (*it)->Disconnect();
}

Now you need to care neither about the locking nor about the iterators validity.