2

I have the following code that deletes the signal during one of the callbacks from the signal:

  #include <iostream>
  #include <boost/signals2/signal.hpp>

  struct Foo {
    boost::signals2::signal<void(int)> signal;
  };

  std::vector<Foo> foos;
  foos.emplace_back(Foo());

  std::vector<int> values;
  auto connection = boost::signals2::scoped_connection(foos[0].signal.connect([&](int x) {
    foos.clear(); // delete the foos here, where we will be calling from below
    values.emplace_back(1);
  }));

  foos[0].signal(1);
  std::cout << "values.size()=" << values.size() << "\n";

Have I just been lucky in this "working" (as in it is undefined behaviour) or is there some magic pointer counting in signals2 that is stopping me from blowing my feet off?

Sigmund Fraud
  • 173
  • 1
  • 7

1 Answers1

1
 // delete the foos here, where we will be calling from below

That's a misleading comment. The deletion (clear()) only happens after raising the signal, so the control flow is in the reverse order from the lines of code.

To me, this code looks valid, unless you destroy connection (e.g. connection.release();) from insde the signal handler. Even then it could be safe, but it would depend on the implementation details of handler iteration. I suspect it will still be fine, because the Signals2 library is expressly thread-aware, so the converse MUST be fine (adding handlers to the same slot during signal invocation).

is there some magic pointer counting in signals2 that is stopping me from blowing my feet off?

The magic reference counting in Signals2 is the bit you already have: scoped_connection, which will automatic disconnect when the last copy goes out of scope.

Not really related to the code shown:

You can of course have the same facility on your own entities, by using things std::vector<shared_ptr<Foo> >. If the references/iterators to elements of foos need to be stable, use std::list or std::deque (with only front/back insertion/removals).

Here's my take on the code which passes UBsan/ASan both with and without optimizations:

Live On Coliru

#include <boost/signals2/signal.hpp>
#include <iostream>

namespace s2 = boost::signals2;

struct Foo {
    s2::signal<void(int)> onEvent;
};

int main() {
    std::vector<Foo> foos(3);

    std::vector<int> values;
    std::cout << "before values.size()=" << values.size() << "\n";

    s2::scoped_connection connection =
        foos.back().onEvent.connect([&](int x) {
            foos.clear(); // delete the foos here
            values.emplace_back(1);
            connection.release();
        });

    foos.back().onEvent(1);
    std::cout << "after values.size()=" << values.size() << "\n";
}

Prints

before values.size()=0
after values.size()=1
sehe
  • 374,641
  • 47
  • 450
  • 633
  • Thank you for your answer. I was worried about the fact the signal destructor is called during the foo.clear(), yet the execution of the callback still continues. I appreciate this is probably worrying about the internals of signals2. I believe the connection release inside the callback is valid due to the way the pointers work inside the implementation (from what I could tell digging about the source). – Sigmund Fraud Jan 15 '22 at 17:42
  • Signals2 is express thread-aware, which informs my understanding that it probably doesn't "accidentally" work right, but has actually been designed for that (as I support with the opposite scenario in my answer) – sehe Jan 15 '22 at 17:44
  • Yes, understood. – Sigmund Fraud Jan 15 '22 at 17:45