Re: [Dbus-cxx-devel] Uninitialized/destroyed mutexes/rwlocks

[Patrick A. <pa...@ph...>]

On Fri, 2009-09-11 at 08:59 -1000, Patrick Allison wrote:
> Think I found the last bug that was causing occasional deadlocks in my
> code using dbus-cxx (turns out it wasn't just the unsent messages).

Haha, that statement was just *asking* for trouble.

There's danger in Dispatcher::watch_thread_main -

      // Now we'll check the fds that are ready for writing and add them
to the set of write fds to handle
      pthread_mutex_lock( &m_mutex_write_watches );
      for ( fditer = m_enabled_write_fds.begin(); fditer !=
m_enabled_write_fds.end(); fditer++ )
      {
        if ( FD_ISSET( *fditer, &write_fds ) )
        {
          witer = m_write_watches.find(*fditer);
          if ( witer != m_write_watches.end() )
            witer->second->handle_write();
        }
      }
      pthread_mutex_unlock( &m_mutex_write_watches );

Here, m_mutex_write_watches is being held over a dbus call, which has a
deadlock risk (thread A takes lock 1 (m_mutex_write_watches), thread A
wakes thread B, thread B takes lock 2 (a mutex inside libdbus), thread B
waits on lock 1, thread A waits on lock 2 - doooom), and there's not
much reason for it. You can always do:

      // Now we'll check the fds that are ready for writing and add them
to the set of write fds to handle
      for ( fditer = m_enabled_write_fds.begin(); fditer !=
m_enabled_write_fds.end(); fditer++ )
      {
        if ( FD_ISSET( *fditer, &write_fds ) )
        {
          pthread_mutex_lock( &m_mutex_write_watches );
          witer = m_write_watches.find(*fditer);          
          if ( witer != m_write_watches.end() ) {
            Watch::pointer wp = witer->second;
            pthread_mutex_unlock( &m_mutex_write_watches );
            wp->handle_write();
          } else {
            pthread_mutex_unlock( &m_mutex_write_watches );
          }
        }
      }

and identically for the read watches. 

Patch attached.

Patrick