Due to odd crashes when cleaning mItems adding thread safety
Viewer runs window in a separate thread, it is possible notification
was added in a way that corrupted the list.
Crash appears to happens inside mItems.clear() of the
LLNotificationChannelBase, but there is no apparent reson for it and
stack jumped some steps, so I'm doing cleanup more explicitly to see
if it's indeed there and not a corruption somewhere earlier.
Mostly converted some boost pointers to std ones
Made ~LLNotificationChannelBase() more explicit to get a bit more data on location of another crash that likely happens when cleaning mItems
Not a fix. Mac sometimes crashes when destroying mItems in LLPersistentNotificationChannel
Decided to try cleaning mItems explicitly to see if it will change callstack, it won't fix the crash, but will help figuring out if source of the issue is in mItems or is LLPersistentNotificationChannel itself
Callstack is clearly broken since it points to LLNotifications::instance().clear(); after 'Goodbye!', my suspicion is that something reinitialized singleton so I fixed cleanup and added some logging to see if there is a dupplicate init
LLEventDetail::visit_and_connect() promised special treatment for the
specific case when an LLEventPump::listen() listener was composed of (possibly
nested) boost::bind() objects storing boost::weak_ptr values -- specifically
boost::bind() rather than std::bind or lambdas, specifically boost::weak_ptr
rather than std::weak_ptr.
Outside of self-tests, it does not appear that anyone actually uses that
support.
There is good reason not to: it's a silent side effect of a complicated
compile-time inspection that could be silently derailed by use of std::bind()
or a lambda or a std::weak_ptr. Can you be sure you've engaged that promise?
How?
A more robust guarantee can be achieved by storing an LLTempBoundConnection in
the transient object itself. When the object is destroyed, the listener is
disconnected. Normal C++ rules around object destruction guarantee it. This
idiom is widely used.
There are a couple good reasons to remove the visit_and_connect() machinery:
* boost::bind() and boost::weak_ptr do not constitute the wave of the future.
Preferring those constructs to lambdas and std::weak_ptr penalizes new code,
whether by silently failing or by discouraging use of modern idioms.
* The visit_and_connect() machinery was always complicated, and apparently
never very robust. Most of its promised features have been commented out
over the years. Making the code base simpler, clearer and more maintainable
is always a useful effect.
LLEventDetail::visit_and_connect() was also used by the four
LLNotificationChannelBase::connectMumble() methods. Streamline those as well.
Of course, remove related test code.
A shocking number of LLSingleton subclasses had public constructors -- and in
several instances, were being explicitly instantiated independently of the
LLSingleton machinery. This breaks the new LLSingleton dependency-tracking
machinery. It seems only fair that if you say you want an LLSingleton, there
should only be ONE INSTANCE!
Introduce LLSINGLETON() and LLSINGLETON_EMPTY_CTOR() macros. These handle the
friend class LLSingleton<whatevah>;
and explicitly declare a private nullary constructor.
To try to enforce the LLSINGLETON() convention, introduce a new pure virtual
LLSingleton method you_must_use_LLSINGLETON_macro() which is, as you might
suspect, defined by the macro. If you declare an LLSingleton subclass without
using LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() in the class body, you can't
instantiate the subclass for lack of a you_must_use_LLSINGLETON_macro()
implementation -- which will hopefully remind the coder.
Trawl through ALL LLSingleton subclass definitions, sprinkling in
LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() as appropriate. Remove all explicit
constructor declarations, public or private, along with relevant 'friend class
LLSingleton<myself>' declarations. Where destructors are declared, move them
into private section as well. Where the constructor was inline but nontrivial,
move out of class body.
Fix several LLSingleton abuses revealed by making ctors/dtors private:
LLGlobalEconomy was both an LLSingleton and the base class for
LLRegionEconomy, a non-LLSingleton. (Therefore every LLRegionEconomy instance
contained another instance of the LLGlobalEconomy "singleton.") Extract
LLBaseEconomy; LLGlobalEconomy is now a trivial subclass of that.
LLRegionEconomy, as you might suspect, now derives from LLBaseEconomy.
LLToolGrab, an LLSingleton, was also explicitly instantiated by
LLToolCompGun's constructor. Extract LLToolGrabBase, explicitly instantiated,
with trivial subclass LLToolGrab, the LLSingleton instance.
(WARNING: LLToolGrabBase methods have an unnerving tendency to go after
LLToolGrab::getInstance(). I DO NOT KNOW what should be the relationship
between the instance in LLToolCompGun and the LLToolGrab singleton instance.)
LLGridManager declared a variant constructor accepting (const std::string&),
with the comment:
// initialize with an explicity grid file for testing.
As there is no evidence of this being called from anywhere, delete it.
LLChicletBar's constructor accepted an optional (const LLSD&). As the LLSD
parameter wasn't used, and as there is no evidence of it being passed from
anywhere, delete the parameter.
LLViewerWindow::shutdownViews() was checking LLNavigationBar::
instanceExists(), then deleting its getInstance() pointer -- leaving a
dangling LLSingleton instance pointer, a land mine if any subsequent code
should attempt to reference it. Use deleteSingleton() instead.
~LLAppViewer() was calling LLViewerEventRecorder::instance() and then
explicitly calling ~LLViewerEventRecorder() on that instance -- leaving the
LLSingleton instance pointer pointing to an allocated-but-destroyed instance.
Use deleteSingleton() instead.
Ansariel Hiller