Is it safe to use a boost::signals2::scoped_connection object as member of a class for automatic connection lifetime?

Question

I wondered whether the following code is safe with respect to the fact that the signal might be triggered by a different thread:

using IntSignal = boost::signals2::signal<void(int)>;

class Foo
{
public:

  Foo(IntSignal& signal)
    : some_resource(new int(0))
  {
    scoped_connection = signal.connect([this](int i) { some_action(i); });
  }

  ~Foo()
  {
    delete some_resource;
  }

private:

  void some_action(int i)
  {
    *some_resource = i;
  }

  int* some_resource;

  boost::signals2::scoped_connection scoped_connection;
}

EDIT: added an imaginary resource, destructor and an implementation for some_action to make it more clear. With this question I would like to clarify whether my assumption is correct that the signal's slot might be called after Foo's destructor but before scoped_connection's destructor. I omitted a mutex protecting some_resource for brevity, however, it is not relevant for the question.

Although the connection will be dropped when a Foo instance is destroyed, there might be a tiny gap betwen Foo's own destructor invocation and the destruction of Foo's members. This might be even more problematic if resources are being used within some_action after they have been destructed.

Should I rather use normal connections and disconnect them in Foo's destructor? And would it be safe to have the scoped_connection member as last member of the class (that should get destroyed first) and omit any destruction logic?

Answer 1

You are right, there is a possible race if Foo's destructor is invoked while the signal is running and accessing some_resource.

A safe solution would be to extend the life of Foo while the slots are running:

class Foo : public std::enable_shared_from_this<Foo>
{
public:

  Foo(IntSignal& signal)
    : some_resource(new int(0))
  {        
      // moved connection initialization to a method since weak_from_this will  
      // be empty inside the constructor. It is initialized only afterwards.
      // It also make sense to connect your signal only after object
      // is fully initialized
  }

  void InitConnection()
  {
     auto weak_self = weak_from_this(); 
     scoped_connection = signal.connect(
       [weak_self](int i) 
      { 
         if (auto self = weak_self.lock())
         {
            // we managed to promote weak_self to a shared_ptr, 'this' will be alive 
            // as long as this scope lives
            some_action(i); // safe
         }
      });

  }

  ~Foo()
  {
     // will only be invoked after Foo's reference count drops to zero.
     // not during the signal is emitted
     delete some_resource; 
  }

private:    
  void some_action(int i)
  {
    *some_resource = i;
  }

  int* some_resource;    
  boost::signals2::scoped_connection scoped_connection;
}

Notes:

1. enable_shared_from_this initializes a weak_ptr to 'this'. It is a great tool for the situation you described. See more here.
2. Make sure you create Foo as a shared_ptr, otherwise weak_from_this will not work. Remember: Foo is shared between 2 threads.