fr00b0/nod

potential data race in emission

NoAvailableAlias opened this issue · 2 comments

Hello, recently I have been working on the next release of nano-signal-slot for some time now and have encountered a data race issue with my current iteration. Curious how others solved it I started looking into other thread safe implementations. Perusing nod I noticed the exact same signature of the issue exists in nod.

The signature is:

for (auto slot : copied_slots)
{
    if (slot)
        slot(args...)
}

The issue is the lock for emission is released as soon as the copied_slots is created. Due to this there is now a "potential data race" where the slot target "could" be destructed before being called in the emission loop.

Edit: Also my first attempt at resolving this is the same as how you currently test your shared disconnector in destruction. However, this would be UB now instead of a data race as accessing a class in destruction is UB.

I think there are two solutions:

  1. Hold the lock during signal emission, recursive mutex can be used if slots need to modify the list.
  2. Make the slot connection class the owner of the std::function and hold a weak_ptr to the connections in the signal class.

Recursive mutex would only allow for reentrancy, the slot emission race would still exist?

Your second point is what my previous Edit: was about.

However, after a year absence, I'm pretty sure there would be no UB as destruction is blocked by its own locked weak_ptr (heap owned reference counter). The block to destruction would only occur if an emission is currently taking place in a different thread as the tracked observer is being destructed.