Disconnect during firing: Expected behaviour?

[v1ne]

Hi. Regardless of the chosen policy, Nano::Signal calls slots which have been disconnected if this happens while firing a signal. Consider:

TEST_CASE("signal-slot nested disconnection") {
  int fireCount = 0;

  Nano::Signal<void(), Nano::ST_Policy_Safe> sig;

  auto disconnectingFirer1 = [&]() {
    ++fireCount;
    sig.disconnect_all();
  };
  auto disconnectingFirer2 = [&]() {
    ++fireCount;
    sig.disconnect_all();
  };

  sig.connect(disconnectingFirer1);
  sig.connect(disconnectingFirer2);
  sig.fire();
  CHECK(fireCount == 1);
}

I'd expect only 1 firing because whoever comes first, disconnects all the others.

Issuing the second callback anyway means that it's tough to use the library when binding to methods of a class (using Nano::Observer for disconnection, of course) to a signal and destroying an instance of that class from a callback bound to that very signal, which is executed first.

What's the expected behaviour for nano-signal-slot?

[NoAvailableAlias]

Hello,

Nano-signal-slot uses an ordered vector for slots which has no guaranteed order of emission. Additionally both of the "_Safe" suffixed policies copy the vector prior to emission. The former reinforces the notion of "slots should be unaware of other slots". The latter has the unfortunate side-effect of outright not allowing slots to modify the "current emission".

The expected behavior for nano-signal-slot must be that slots cannot modify the "current emission" due to these two design constraints.

Concerning class methods, the ST_Policy_Safe is only to be used in single threaded code which has deterministic object lifetimes. The TS_Policy_Safe uses a weak_ptr to track the lifetime of each class method and is locked in emission.

I'll have to update both the readme and the 2.0 release info in regards to "emission ordering" as that is a big change between 1.0 and 2.0.

[v1ne]

Hi. Indeed, that's why I also I designed the test to not assume any order of submission. ;)

Adding a paragraph about the design philosophy sounds like a good idea!

So if I use TS_Policy_Safe, this would not access dead memory anymore in the case of calling member functions, I see. Just that if the class instance doesn't die, but disconnects during an emission, the signal would still be delivered.

Thank you!

[NoAvailableAlias]

Thanks for the question, found a couple missing things from the readme as a result. I'll also take a look into the Test_Signal_Edge_Cases to see if it needs any additions.