From 35ee96709ef2704a2636a11c67d61190dd6bdd50 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 18 Jul 2024 12:55:38 -0400 Subject: Make `LLEventPump::listen()` also accept new `LLAwareListener`. `listen()` still takes `LLEventListener`, a `callable(const LLSD&)`, but now also accepts `LLAwareListener`, a `callable(const LLBoundListener&, const LLSD&)`. This uses `boost::signals2::signal::connect_extended()`, which, when the signal is called, passes to a connected listener the `LLBoundListener` (aka `boost::signals2::connection`) representing its own connection. This allows a listener to disconnect itself when done. Internally, `listen_impl()` now always uses `connect_extended()`. When passed a classic `LLEventListener`, `listen()` wraps it in a lambda that ignores the passed `LLBoundListener`. `listen()` also now accepts `LLVoidListener`, and internally wraps it in a lambda that returns `false` on its behalf. --- indra/llcommon/llevents.h | 70 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 9 deletions(-) (limited to 'indra/llcommon/llevents.h') diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index d0686bd8b5..8ef3a5af95 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -32,12 +32,13 @@ #if ! defined(LL_LLEVENTS_H) #define LL_LLEVENTS_H -#include +#include +#include #include #include +#include +#include #include -#include -#include #if LL_WINDOWS #pragma warning (push) #pragma warning (disable : 4263) // boost::signals2::expired_slot::what() has const mismatch @@ -137,6 +138,10 @@ typedef boost::signals2::signal LL /// Methods that forward listeners (e.g. constructed with /// boost::bind()) should accept (const LLEventListener&) typedef LLStandardSignal::slot_type LLEventListener; +/// Support a listener accepting (const LLBoundListener&, const LLSD&). +/// Note that LLBoundListener::disconnect() is a const method: this feature is +/// specifically intended to allow a listener to disconnect itself when done. +typedef LLStandardSignal::extended_slot_type LLAwareListener; /// Accept a void listener too typedef std::function LLVoidListener; /// Result of registering a listener, supports connected(), @@ -522,18 +527,37 @@ public: * call, allows us to optimize away the second and subsequent dependency * sorts. * - * If name is set to LLEventPump::ANONYMOUS listen will bypass the entire + * If name is set to LLEventPump::ANONYMOUS, listen() will bypass the entire * dependency and ordering calculation. In this case, it is critical that * the result be assigned to a LLTempBoundListener or the listener is - * manually disconnected when no longer needed since there will be no + * manually disconnected when no longer needed, since there will be no * way to later find and disconnect this listener manually. */ + template LLBoundListener listen(const std::string& name, - const LLEventListener& listener, + LISTENER&& listener, const NameList& after=NameList(), const NameList& before=NameList()) { - return listen_impl(name, listener, after, before); + if constexpr (std::is_invocable_v) + { + // wrap classic LLEventListener in LLAwareListener lambda + return listenb( + name, + [listener=std::move(listener)] + (const LLBoundListener&, const LLSD& event) + { + return listener(event); + }, + after, + before); + } + else + { + static_assert(std::is_invocable_v, + "LLEventPump::listen() listener has bad parameter signature"); + return listenb(name, std::forward(listener), after, before); + } } /// Get the LLBoundListener associated with the passed name (dummy @@ -579,7 +603,35 @@ private: LLMutex mConnectionListMutex; protected: - virtual LLBoundListener listen_impl(const std::string& name, const LLEventListener&, + template + LLBoundListener listenb(const std::string& name, + LISTENER&& listener, + const NameList& after=NameList(), + const NameList& before=NameList()) + { + using result_t = std::decay_t; + if constexpr (std::is_same_v) + { + return listen_impl(name, std::forward(listener), after, before); + } + else + { + static_assert(std::is_same_v, + "LLEventPump::listen() listener has bad return type"); + // wrap void listener in one that returns bool + return listen_impl( + name, + [listener=std::move(listener)] + (const LLBoundListener& conn, const LLSD& event) + { + listener(conn, event); + return false; + }, + after, + before); + } + } + virtual LLBoundListener listen_impl(const std::string& name, const LLAwareListener&, const NameList& after, const NameList& before); @@ -654,7 +706,7 @@ public: void discard(); protected: - virtual LLBoundListener listen_impl(const std::string& name, const LLEventListener&, + virtual LLBoundListener listen_impl(const std::string& name, const LLAwareListener&, const NameList& after, const NameList& before) override; -- cgit v1.2.3 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/llcommon/llevents.h | 45 +-------------------------------------------- 1 file changed, 1 insertion(+), 44 deletions(-) (limited to 'indra/llcommon/llevents.h') diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 8ef3a5af95..abc25ba400 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -363,57 +363,14 @@ testable: InstanceTypes mTypes; }; -/***************************************************************************** -* LLEventTrackable -*****************************************************************************/ -/** - * LLEventTrackable wraps boost::signals2::trackable, which resembles - * boost::trackable. Derive your listener class from LLEventTrackable instead, - * and use something like - * LLEventPump::listen(boost::bind(&YourTrackableSubclass::method, - * instance, _1)). This will implicitly disconnect when the object - * referenced by @c instance is destroyed. - * - * @note - * LLEventTrackable doesn't address a couple of cases: - * * Object destroyed during call - * - You enter a slot call in thread A. - * - Thread B destroys the object, which of course disconnects it from any - * future slot calls. - * - Thread A's call uses 'this', which now refers to a defunct object. - * Undefined behavior results. - * * Call during destruction - * - @c MySubclass is derived from LLEventTrackable. - * - @c MySubclass registers one of its own methods using - * LLEventPump::listen(). - * - The @c MySubclass object begins destruction. ~MySubclass() - * runs, destroying state specific to the subclass. (For instance, a - * Foo* data member is deleted but not zeroed.) - * - The listening method will not be disconnected until - * ~LLEventTrackable() runs. - * - Before we get there, another thread posts data to the @c LLEventPump - * instance, calling the @c MySubclass method. - * - The method in question relies on valid @c MySubclass state. (For - * instance, it attempts to dereference the Foo* pointer that was - * deleted but not zeroed.) - * - Undefined behavior results. - */ -typedef boost::signals2::trackable LLEventTrackable; - /***************************************************************************** * LLEventPump *****************************************************************************/ /** * LLEventPump is the base class interface through which we access the * concrete subclasses such as LLEventStream. - * - * @NOTE - * LLEventPump derives from LLEventTrackable so that when you "chain" - * LLEventPump instances together, they will automatically disconnect on - * destruction. Please see LLEventTrackable documentation for situations in - * which this may be perilous across threads. */ -class LL_COMMON_API LLEventPump: public LLEventTrackable +class LL_COMMON_API LLEventPump { public: static const std::string ANONYMOUS; // constant for anonymous listeners. -- cgit v1.2.3