From f2f0fa7fd0efc221f1358dd4e440b5d51a5fb8b4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 18 Jul 2024 13:29:34 -0400 Subject: Ditch `LLEventTrackable` aka `boost::signals2::trackable`. Remove documented `LLEventPump` support for `LLEventTrackable`. That claimed support was always a little bit magical/fragile. IF: * a class included `LLEventTrackable` as a base class AND * an instance of that class was managed by `boost::shared_ptr` AND * you passed one of that class's methods and the `boost::shared_ptr` specifically to `boost::bind()` AND * the resulting `boost::bind()` object was passed into `LLEventPump::listen()` THEN the promise was that on destruction of that object, that listener would automatically be disconnected -- instead of leaving a dangling pointer bound into the `LLEventPump`, causing a crash on the next `LLEventPump::post()` call. The only existing code in the viewer code base that exercised `LLEventTrackable` functionality was in test programs. When the viewer calls `LLEventPump::listen()`, it typically stores the resulting connection object in an `LLTempBoundListener` variable, which guarantees disconnection on destruction of that variable. The fact that `LLEventTrackable` support is specific to `boost::bind()`, that it silently fails to keep its promise with `std::bind()` or a lambda or any other form of C++ callable, makes it untrustworthy for new code. Note that the code base still uses `boost::signals2::trackable` for other `boost::signals2::signal` instances not associated with `LLEventPump`. We are not changing those at this time. --- indra/llui/llnotifications.h | 2 +- indra/llui/llnotificationslistener.cpp | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) (limited to 'indra/llui') diff --git a/indra/llui/llnotifications.h b/indra/llui/llnotifications.h index ab4f009a80..206c521592 100644 --- a/indra/llui/llnotifications.h +++ b/indra/llui/llnotifications.h @@ -734,7 +734,7 @@ typedef std::multimap LLNotificationMap; // all of the built-in tests should attach to the "Visible" channel // class LLNotificationChannelBase : - public LLEventTrackable, + public boost::signals2::trackable, public LLRefCount { LOG_CLASS(LLNotificationChannelBase); diff --git a/indra/llui/llnotificationslistener.cpp b/indra/llui/llnotificationslistener.cpp index ace9e37e25..2ad1689a45 100644 --- a/indra/llui/llnotificationslistener.cpp +++ b/indra/llui/llnotificationslistener.cpp @@ -204,7 +204,7 @@ void LLNotificationsListener::ignore(const LLSD& params) const } } -class LLNotificationsListener::Forwarder: public LLEventTrackable +class LLNotificationsListener::Forwarder: public boost::signals2::trackable { LOG_CLASS(LLNotificationsListener::Forwarder); public: @@ -213,8 +213,10 @@ public: mRespond(false) { // Connect to the specified channel on construction. Because - // LLEventTrackable is a base, we should automatically disconnect when - // destroyed. + // boost::signals2::trackable is a base, because we use boost::bind() + // below, and because connectPassedFilter() directly calls + // boost::signals2::signal::connect(), we should automatically + // disconnect when destroyed. LLNotificationChannelPtr channelptr(llnotifications.getChannel(channel)); if (channelptr) { @@ -252,10 +254,10 @@ void LLNotificationsListener::forward(const LLSD& params) if (! forward) { // This is a request to stop forwarding notifications on the specified - // channel. The rest of the params don't matter. - // Because mForwarders contains scoped_ptrs, erasing the map entry - // DOES delete the heap Forwarder object. Because Forwarder derives - // from LLEventTrackable, destroying it disconnects it from the + // channel. The rest of the params don't matter. Because mForwarders + // contains scoped_ptrs, erasing the map entry DOES delete the heap + // Forwarder object. Because Forwarder derives from + // boost::signals2::trackable, destroying it disconnects it from the // channel. mForwarders.erase(channel); return; -- cgit v1.2.3