Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#16 design of sigslot is broken for multithreaded programs

open
nobody
None
5
2014-03-31
2011-02-03
Alak Noke
No

Hi all,

Be aware that this library is not safe for use in a multithreaded application.

It contains synchronization but its design makes it impossible to use it safely easily (as described in its documentation).
Destructor of has_slots base class is called after the destructor of the derived class. So just image that a slot of the has_slots derived class is called (from a different thread) while you are in the middle of the destructor of the derived class (so the slot may e.g use a member pointer which points to an object which was already destroyed). The sigslot framework/library does not have any chance to avoid this call as it will know about the destruction only when the base class (has_slots) destructor is called.

You may call the disconnect_all at the beginning of your destructor but in this case it is quite useless to derive your class has_slots.

BR,
Alak

Discussion

  • Nice finding!
    So many years left behind ;-)
    Any idea how fix it globally except calling disconnect_all() from derived class's destructor?

     

  • Anonymous
    2011-07-06

    As I can see, this lib is using libsigc++/Boost.Signals model for automatic connection management (see "trackable" class in both of them).
    Maybe it's better to add shared_ptr/weak_ptr-based tracking as in Boost.Signals2. See http://www.boost.org/doc/libs/release/doc/html/signals2/thread-safety.html

    Not that I'm a fan of Boost.Signals2, no. It's huge, it's overengineered, it doesn't comply with the well-known "you only pay for what you use" motto, it bloats your executable.
    But it has a nice thread-safe slot tracking system. =)

     
  • Davy Durham
    Davy Durham
    2014-03-31

    calling disconnect_all() from a derived class does help.

    However, if you notice, doing can lead to dead-lock since disconnect_all locks the has_slots' mutex first, then each signal's mutex.

    But when connecting() it locks the signal's mutex first and has_slots' second.

    Subtle, and unlikley, but easy to fix by changing sigslot code to not hold the signal's mutex in connect() when it tells the has_slots about it self.

     
    Last edit: Davy Durham 2014-03-31