Montellese / cpp-signal

Header-only pure C++11 library providing signal and slot functionality
MIT License
17 stars 6 forks source link

Library is not thread-safe: deadlock can happen #2

Open sergey-shambir opened 6 years ago

sergey-shambir commented 6 years ago

Here library calls foreign callbacks under mutex lock:

template<class TSlot, typename... TCallArgs>
    void call(TCallArgs&&... args)
    {
      lock_guard lock(*this);
      for (const auto& slot : slots_)
      {
        if (!slot.call)
          continue;

        TSlot(slot.key).call(std::forward<TCallArgs>(args)...);
      }
    }

    template<class TInit, class TSlot, typename... TCallArgs>
    TInit accumulate(TInit&& init, TCallArgs&&... args)
    {
      static_assert(std::is_same<typename TSlot::result_type, void>::value == false, "Cannot accumulate slot return values of type 'void'");

      lock_guard lock(*this);
      for (const auto& slot : slots_)
      {
        if (!slot.call)
          continue;

        init = init + TSlot(slot.key).call(std::forward<TCallArgs>(args)...);
      }

      return init;
}

Imagine that one thread connects to "signal1" own function "slot1" which fires "signal2". First, it locks "signal1" mutex, then attempts to lock "signal2" mutex.

Another thread connects to "signal2" own function "slot2" which fires "signal1". First, it locks "signal2" mutex, then attempts to lock "signal1" mutex.

In some cases, threads will enter into deadlock: thread1 always waits "signal2" mutex, while thread2 always waits "signal1" mutex.

sergey-shambir commented 6 years ago

I've illustrated problem: default

Montellese commented 5 years ago

This sounds more like a "design" problem in your application which will always lead to deadlocks when using locks / mutexes.

If signal1 triggers slot1 which fires signal2 which triggers slot2 which fires signal1 which again triggers slot1 you've got yourself a nice indirect endless loop.

sergey-shambir commented 5 years ago

Code above just shows the issue; of course, in real code there will be conditional execution and more levels of indirection, so no endless loops will appear. But deadlock is still possible when library calls external code which can again call library API which acquires another lock. The same thing happens in the "Dining philosophers problem".

Montellese commented 5 years ago

I have added a "Known Issues" section to the README mentioning this potential deadlock.