stored weak_ptr of track object will prevent the track object memory being freed

[Lutts Cao <lutts.cao@…>]

boost signals2 will store weak_ptr of a tracked object, if the tracked object is allocated by std::make_shared() or std::allocate_shared(), after it is destructed, the memory occupied by it will not be freed because the weak_ptr is still hold by the signal connection. Only when the signal is re-triggered will the memory being freed.

there's another bug here, after the tracked object is destructed, signals2::connection.connected() will return false, and connection.disconnect() seem does nothing when connected is false, and leave the tracked object memory leaked.

below is some test cases

#include <iostream>  // std::cout
#include <memory>    // std::shared_ptr
#include <functional>  // std::function

#include <boost/signals2.hpp>

class TrackObject {
 public:
  TrackObject() { std::cout << "construct TrackObject" << std::endl; }
  ~TrackObject() { std::cout << "destruct TrackObject" << std::endl; }

  static void* operator new(std::size_t sz) {
    void* p = ::operator new(sz);
    std::cout << "custom new for size " << sz << ", got p " << p << std::endl;
    return p;
  }

  static void operator delete(void* p) {
    std::cout << "custom delete for p " << p << std::endl;
    return ::operator delete(p);
  }
  static void* operator new[](std::size_t sz) {
    std::cout << "custom new for size " << sz << std::endl;
    return ::operator new(sz);
  }
 private:
  char data[50];
};

class SignalEmitter {
 public:
  using SignalType = boost::signals2::signal<void()>;

  SignalEmitter() = default;
  virtual ~SignalEmitter() = default;

  boost::signals2::connection
  registerHandler(std::function<void()> func,
                  std::shared_ptr<TrackObject> trackobj) {
    SignalType::slot_type handler(func);
    return signal_.connect(handler.track_foreign(trackobj));
  }

  void trigger()  {
    signal_();
  }

 private:
  SignalType signal_;
};

template <class T>
struct custom_allocator {
  typedef T value_type;
  custom_allocator() noexcept {}
  template <class U> custom_allocator(const custom_allocator<U>&) noexcept {}
  T* allocate(std::size_t n) {
    std::cout << "allocate " << n
              << " objects, object size is " << sizeof(T) << std::endl;
    return static_cast<T*>(::operator new(n * sizeof(T)));
  }
  void deallocate(T* p, std::size_t n) {
    std::cout << "deallocate " << n << " objects" << std::endl;
    ::operator delete(p);
  }
};

int main() {
  std::cout << "---TestCase1: check wp will hold the memory---" << std::endl;

  custom_allocator<TrackObject> alloc;
  {
    std::weak_ptr<TrackObject> wp4;

    {
      auto p4 = std::allocate_shared<TrackObject>(alloc);
      wp4 = p4;
    }
    std::cout << "---TestCase1: expect to free the memory before this line, "
              << "but wp prevents it from being freed ---"
              << std::endl;
  }
  std::cout
      << "---TestCase1: the memory is only freed when weak_ptr is out of scope"
      << ", before this line ---"
      << std::endl;

  std::cout
      << "---TestCase2: check signal2 will prevent memory from being freed---"
      << std::endl;
  {
    SignalEmitter signal_emiiter;
    boost::signals2::connection conn;
    {
      auto trackobj = std::allocate_shared<TrackObject>(alloc);

      conn = signal_emiiter.registerHandler(
          []() {
            std::cout << "signal triggered" << std::endl;
          }, trackobj);
      std::cout << "---TestCase2: after registerHandler: conn.connected: "
                << conn.connected()
                << std::endl;
      signal_emiiter.trigger();
    }
    std::cout << "---TestCase2: should free trackobj here, but not---\n";
    std::cout << "---TestCase2: connected: " << conn.connected() << std::endl;
    std::cout
        << "---TestCase2: connected() shows that it's already disconnected"
        << std::endl;
    conn.disconnect();
    std::cout << "--TestCase2: so, conn.disconnect() does nothing, "
        "the memory is still not freed" << std::endl;
    signal_emiiter.trigger();
    std::cout
        << "---TestCase2: only re-trigger will free memory before this line ---"
        << std::endl;
  }

  std::cout << "---TestCase3: check non-make_shared workaround---" << std::endl;
  {
    SignalEmitter signal_emiiter;
    {
      std::shared_ptr<TrackObject> trackobj(new TrackObject());

      signal_emiiter.registerHandler(
          [](){
            std::cout << "signal triggered" << std::endl;
          }, trackobj);
      signal_emiiter.trigger();
    }
    std::cout << "---TestCase3: without use make_shared, memory will be "
        "freed before this line---" << std::endl;
    signal_emiiter.trigger();
    std::cout << "---TestCase3: re-trigger do nothing ---" << std::endl;
  }
}

[Lutts Cao <lutts.cao@…>]

nolock_cleanup_connections_from() is only called by connect() and operator(), I think there's need to add an explicit API to boost::signals2::signal to let the user explicitly cleanup expired connections

[Frank Mori Hess]

You may be happier with signals2 from boost 1.59. It deletes slot objects inside the signal (and their tracked weak_ptrs) ASAP after slot disconnection, rather than putting it off for the signal to garbage collect at some later point.

While weak_ptr causing a delay in the deallocation of memory for shared_ptr created with allocate_shared is interesting, it is an issue with allocate_shared, not signals2. My guess is allocate_shared allocates a single block of memory to contain both the pointed-at object and the shared reference count, in order to minimize the number of memory allocations. Thus the memory cannot be freed until both the pointed-at object and the shared reference count are destroyed. Since the weak_ptr has a handle to the shared reference count, it delays the deallocation.

I'm closing this ticket, if there is still a need for an added cleanup API when using signals2 from 1.59, please open a separate ticket for that feature request.