viewer#2985 Odd crash when notifying voice observers

Crash points at enableDisableCallBtn, but button's floater was cleaned
earlier and has a removeObserver call so an observer record should be
long gone. Likely something else is going on.
Went over various callbacks and made sure they are cleaned. But in case
floater somehow remained, added mSession = nullptr.

15 files changed, 86 insertions, 38 deletions

diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index 093314a9f1..08b7b439e8 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -5151,10 +5151,7 @@ void LLAppViewer::sendLogoutRequest()
         gLogoutMaxTime = LOGOUT_REQUEST_TIME;
         mLogoutRequestSent = true;
 
-        if(LLVoiceClient::instanceExists())
-        {
-            LLVoiceClient::getInstance()->setVoiceEnabled(false);
-        }
+        LLVoiceClient::setVoiceEnabled(false);
     }
 }
 
diff --git a/indra/newview/llchiclet.cpp b/indra/newview/llchiclet.cpp
index 4c0f160f6f..e8a069dfdb 100644
--- a/indra/newview/llchiclet.cpp
+++ b/indra/newview/llchiclet.cpp
@@ -511,7 +511,7 @@ bool LLChicletPanel::postBuild()
     LLScriptFloaterManager::getInstance()->addNewObjectCallback(boost::bind(&LLChicletPanel::objectChicletCallback, this, _1));
     LLScriptFloaterManager::getInstance()->addToggleObjectFloaterCallback(boost::bind(&LLChicletPanel::objectChicletCallback, this, _1));
     LLIMChiclet::sFindChicletsSignal.connect(boost::bind(&LLChicletPanel::findChiclet<LLChiclet>, this, _1));
-    LLVoiceChannel::setCurrentVoiceChannelChangedCallback(boost::bind(&LLChicletPanel::onCurrentVoiceChannelChanged, this, _1));
+    mVoiceChannelChanged = LLVoiceChannel::setCurrentVoiceChannelChangedCallback(boost::bind(&LLChicletPanel::onCurrentVoiceChannelChanged, this, _1));
 
     mLeftScrollButton=getChild<LLButton>("chicklet_left_scroll_button");
     LLTransientFloaterMgr::getInstance()->addControlView(mLeftScrollButton);
diff --git a/indra/newview/llchiclet.h b/indra/newview/llchiclet.h
index ce8968253b..5114c97beb 100644
--- a/indra/newview/llchiclet.h
+++ b/indra/newview/llchiclet.h
@@ -822,6 +822,8 @@ protected:
     S32 mMinWidth;
     bool mShowControls;
     static const S32 s_scroll_ratio;
+
+    boost::signals2::connection mVoiceChannelChanged;
 };
 
 template<class T>
diff --git a/indra/newview/llconversationlog.cpp b/indra/newview/llconversationlog.cpp
index ed563cbec9..e6a720e734 100644
--- a/indra/newview/llconversationlog.cpp
+++ b/indra/newview/llconversationlog.cpp
@@ -190,6 +190,22 @@ LLConversationLog::LLConversationLog() :
 {
 }
 
+LLConversationLog::~LLConversationLog()
+{
+    if (mLoggingEnabled)
+    {
+        if (LLIMMgr::instanceExists())
+        {
+            LLIMMgr::instance().removeSessionObserver(this);
+        }
+        LLAvatarTracker::instance().removeObserver(mFriendObserver);
+    }
+    if (mAvatarNameCacheConnection.connected())
+    {
+        mAvatarNameCacheConnection.disconnect();
+    }
+}
+
 void LLConversationLog::enableLogging(S32 log_mode)
 {
     mLoggingEnabled = log_mode > 0;
diff --git a/indra/newview/llconversationlog.h b/indra/newview/llconversationlog.h
index b3a5be321e..957d90d55e 100644
--- a/indra/newview/llconversationlog.h
+++ b/indra/newview/llconversationlog.h
@@ -166,13 +166,7 @@ public:
 
 private:
 
-    virtual ~LLConversationLog()
-    {
-        if (mAvatarNameCacheConnection.connected())
-        {
-            mAvatarNameCacheConnection.disconnect();
-        }
-    }
+    virtual ~LLConversationLog();
 
     void enableLogging(S32 log_mode);
 
diff --git a/indra/newview/llfloaterimsession.cpp b/indra/newview/llfloaterimsession.cpp
index 97e0d01b52..185274981b 100644
--- a/indra/newview/llfloaterimsession.cpp
+++ b/indra/newview/llfloaterimsession.cpp
@@ -638,6 +638,7 @@ void LLFloaterIMSession::onClose(bool app_quitting)
     // Last change:
     // EXT-3516 X Button should end IM session, _ button should hide
     gIMMgr->leaveSession(mSessionID);
+    mSession = nullptr; // leaveSession should have deleted it.
     // *TODO: Study why we need to restore the floater before we close it.
     // Might be because we want to save some state data in some clean open state.
     LLFloaterIMSessionTab::restoreFloater();
diff --git a/indra/newview/llfloaterimsessiontab.cpp b/indra/newview/llfloaterimsessiontab.cpp
index fe0916bf15..af38d696bc 100644
--- a/indra/newview/llfloaterimsessiontab.cpp
+++ b/indra/newview/llfloaterimsessiontab.cpp
@@ -80,6 +80,7 @@ LLFloaterIMSessionTab::LLFloaterIMSessionTab(const LLSD& session_id)
 {
     setAutoFocus(false);
     mSession = LLIMModel::getInstance()->findIMSession(mSessionID);
+    LLIMMgr::instance().addSessionObserver(this);
 
     mCommitCallbackRegistrar.add("IMSession.Menu.Action",
             boost::bind(&LLFloaterIMSessionTab::onIMSessionMenuItemClicked,  this, _2));
@@ -102,6 +103,7 @@ LLFloaterIMSessionTab::LLFloaterIMSessionTab(const LLSD& session_id)
 LLFloaterIMSessionTab::~LLFloaterIMSessionTab()
 {
     delete mRefreshTimer;
+    LLIMMgr::instance().removeSessionObserver(this);
 
     LLFloaterIMContainer* im_container = LLFloaterIMContainer::findInstance();
     if (im_container)
@@ -440,7 +442,10 @@ void LLFloaterIMSessionTab::enableDisableCallBtn()
 
     bool enable = false;
 
-    if (mSessionID.notNull() && mSession && mSession->mSessionInitialized && mSession->mCallBackEnabled)
+    if (mSessionID.notNull()
+        && mSession
+        && mSession->mSessionInitialized
+        && mSession->mCallBackEnabled)
     {
         if (mVoiceButtonHangUpMode)
         {
@@ -450,9 +455,10 @@ void LLFloaterIMSessionTab::enableDisableCallBtn()
         else
         {
             // We allow to start call from this state only
-            if (mSession->mVoiceChannel  &&
-                !mSession->mVoiceChannel->callStarted() &&
-                LLVoiceClient::instanceExists())
+            if (LLVoiceClient::instanceExists() &&
+                mSession->mVoiceChannel  &&
+                !mSession->mVoiceChannel->callStarted()
+                )
             {
                 LLVoiceClient* client = LLVoiceClient::getInstance();
                 if (client->voiceEnabled() && client->isVoiceWorking())
@@ -1366,6 +1372,14 @@ LLView* LLFloaterIMSessionTab::getChatHistory()
     return mChatHistory;
 }
 
+void LLFloaterIMSessionTab::sessionRemoved(const LLUUID& session_id)
+{
+    if (session_id == mSessionID)
+    {
+        mSession = nullptr;
+    }
+}
+
 bool LLFloaterIMSessionTab::handleKeyHere(KEY key, MASK mask )
 {
     bool handled = false;
diff --git a/indra/newview/llfloaterimsessiontab.h b/indra/newview/llfloaterimsessiontab.h
index bee5c8c2c4..367d988f26 100644
--- a/indra/newview/llfloaterimsessiontab.h
+++ b/indra/newview/llfloaterimsessiontab.h
@@ -45,6 +45,7 @@ class LLPanelEmojiComplete;
 
 class LLFloaterIMSessionTab
     : public LLTransientDockableFloater
+    , public LLIMSessionObserver
 {
     using super = LLTransientDockableFloater;
 
@@ -76,13 +77,13 @@ public:
     bool isNearbyChat() {return mIsNearbyChat;}
 
     // LLFloater overrides
-    /*virtual*/ void onOpen(const LLSD& key);
-    /*virtual*/ bool postBuild();
-    /*virtual*/ void draw();
-    /*virtual*/ void setVisible(bool visible);
-    /*virtual*/ void setFocus(bool focus);
-    /*virtual*/ void closeFloater(bool app_quitting = false);
-    /*virtual*/ void deleteAllChildren();
+    void onOpen(const LLSD& key) override;
+    bool postBuild() override;
+    void draw() override;
+    void setVisible(bool visible) override;
+    void setFocus(bool focus) override;
+    void closeFloater(bool app_quitting = false) override;
+    void deleteAllChildren() override;
 
     // Handle the left hand participant list widgets
     void addConversationViewParticipant(LLConversationItem* item, bool update_view = true);
@@ -98,7 +99,7 @@ public:
     virtual void updateMessages() {}
     LLConversationItem* getCurSelectedViewModelItem();
     void forceReshape();
-    virtual bool handleKeyHere( KEY key, MASK mask );
+    virtual bool handleKeyHere( KEY key, MASK mask ) override;
     bool isMessagePaneExpanded(){return mMessagePaneExpanded;}
     void setMessagePaneExpanded(bool expanded){mMessagePaneExpanded = expanded;}
     void restoreFloater();
@@ -108,6 +109,13 @@ public:
 
     LLView* getChatHistory();
 
+    // LLIMSessionObserver triggers
+    virtual void sessionAdded(const LLUUID& session_id, const std::string& name, const LLUUID& other_participant_id, bool has_offline_msg) override {}; // Stub
+    virtual void sessionActivated(const LLUUID& session_id, const std::string& name, const LLUUID& other_participant_id) override {}; // Stub
+    virtual void sessionRemoved(const LLUUID& session_id) override;
+    virtual void sessionVoiceOrIMStarted(const LLUUID& session_id) override {};                              // Stub
+    virtual void sessionIDUpdated(const LLUUID& old_session_id, const LLUUID& new_session_id) override {};   // Stub
+
 protected:
 
     // callback for click on any items of the visual states menu
@@ -139,8 +147,8 @@ protected:
     virtual void enableDisableCallBtn();
 
     // process focus events to set a currently active session
-    /* virtual */ void onFocusReceived();
-    /* virtual */ void onFocusLost();
+    void onFocusReceived() override;
+    void onFocusLost() override;
 
     // prepare chat's params and out one message to chatHistory
     void appendMessage(const LLChat& chat, const LLSD& args = LLSD());
@@ -212,7 +220,7 @@ private:
     void getSelectedUUIDs(uuid_vec_t& selected_uuids);
 
     /// Refreshes the floater at a constant rate.
-    virtual void refresh() = 0;
+    virtual void refresh() override = 0;
 
     /**
      * Adjusts chat history height to fit vertically with input chat field
diff --git a/indra/newview/llimview.cpp b/indra/newview/llimview.cpp
index 06cf9919b6..34a4b5b230 100644
--- a/indra/newview/llimview.cpp
+++ b/indra/newview/llimview.cpp
@@ -796,7 +796,6 @@ LLIMModel::LLIMSession::LLIMSession(const LLUUID& session_id,
 
 void LLIMModel::LLIMSession::initVoiceChannel(const LLSD& voiceChannelInfo)
 {
-
     if (mVoiceChannel)
     {
         if (mVoiceChannel->isThisVoiceChannel(voiceChannelInfo))
@@ -2344,7 +2343,7 @@ LLCallDialogManager::~LLCallDialogManager()
 
 void LLCallDialogManager::initSingleton()
 {
-    LLVoiceChannel::setCurrentVoiceChannelChangedCallback(LLCallDialogManager::onVoiceChannelChanged);
+    mVoiceChannelChanged = LLVoiceChannel::setCurrentVoiceChannelChangedCallback(LLCallDialogManager::onVoiceChannelChanged);
 }
 
 // static
diff --git a/indra/newview/llimview.h b/indra/newview/llimview.h
index 99b19c9fa9..61776860e3 100644
--- a/indra/newview/llimview.h
+++ b/indra/newview/llimview.h
@@ -548,6 +548,8 @@ protected:
     std::string mCurrentSessionlName;
     LLIMModel::LLIMSession* mSession;
     LLVoiceChannel::EState mOldState;
+
+    boost::signals2::connection mVoiceChannelChanged;
 };
 
 class LLCallDialog : public LLDockableFloater
diff --git a/indra/newview/llspeakingindicatormanager.cpp b/indra/newview/llspeakingindicatormanager.cpp
index a9ef5e244d..532b245ced 100644
--- a/indra/newview/llspeakingindicatormanager.cpp
+++ b/indra/newview/llspeakingindicatormanager.cpp
@@ -133,6 +133,8 @@ private:
      * @see onChange()
      */
     speaker_ids_t mSwitchedIndicatorsOn;
+
+    boost::signals2::connection mVoiceChannelChanged;
 };
 
 //////////////////////////////////////////////////////////////////////////
@@ -181,7 +183,7 @@ void SpeakingIndicatorManager::unregisterSpeakingIndicator(const LLUUID& speaker
 //////////////////////////////////////////////////////////////////////////
 SpeakingIndicatorManager::SpeakingIndicatorManager()
 {
-    LLVoiceChannel::setCurrentVoiceChannelChangedCallback(boost::bind(&SpeakingIndicatorManager::sOnCurrentChannelChanged, this, _1));
+    mVoiceChannelChanged = LLVoiceChannel::setCurrentVoiceChannelChangedCallback(boost::bind(&SpeakingIndicatorManager::sOnCurrentChannelChanged, this, _1));
     LLVoiceClient::addObserver(this);
 }
 
diff --git a/indra/newview/llvoicechannel.cpp b/indra/newview/llvoicechannel.cpp
index 55769f567b..cf128f381a 100644
--- a/indra/newview/llvoicechannel.cpp
+++ b/indra/newview/llvoicechannel.cpp
@@ -80,9 +80,8 @@ LLVoiceChannel::~LLVoiceChannel()
     if (sCurrentVoiceChannel == this)
     {
         sCurrentVoiceChannel = NULL;
-        // Must check instance exists here, the singleton MAY have already been destroyed.
-        LLVoiceClient::removeObserver(this);
     }
+    LLVoiceClient::removeObserver(this);
 
     sVoiceChannelMap.erase(mSessionID);
 }
diff --git a/indra/newview/llvoiceclient.cpp b/indra/newview/llvoiceclient.cpp
index 1a35a71706..623d8deac9 100644
--- a/indra/newview/llvoiceclient.cpp
+++ b/indra/newview/llvoiceclient.cpp
@@ -171,7 +171,11 @@ void LLVoiceClient::init(LLPumpIO *pump)
 
 void LLVoiceClient::userAuthorized(const std::string& user_id, const LLUUID &agentID)
 {
-    gAgent.addRegionChangedCallback(boost::bind(&LLVoiceClient::onRegionChanged, this));
+    if (mRegionChangedCallbackSlot.connected())
+    {
+        mRegionChangedCallbackSlot.disconnect();
+    }
+    mRegionChangedCallbackSlot = gAgent.addRegionChangedCallback(boost::bind(&LLVoiceClient::onRegionChanged, this));
     LLWebRTCVoiceClient::getInstance()->userAuthorized(user_id, agentID);
     LLVivoxVoiceClient::getInstance()->userAuthorized(user_id, agentID);
 }
@@ -608,8 +612,14 @@ bool LLVoiceClient::voiceEnabled()
 
 void LLVoiceClient::setVoiceEnabled(bool enabled)
 {
-    LLWebRTCVoiceClient::getInstance()->setVoiceEnabled(enabled);
-    LLVivoxVoiceClient::getInstance()->setVoiceEnabled(enabled);
+    if (LLWebRTCVoiceClient::instanceExists())
+    {
+        LLWebRTCVoiceClient::getInstance()->setVoiceEnabled(enabled);
+    }
+    if (LLVivoxVoiceClient::instanceExists())
+    {
+        LLVivoxVoiceClient::getInstance()->setVoiceEnabled(enabled);
+    }
 }
 
 void LLVoiceClient::updateMicMuteLogic()
diff --git a/indra/newview/llvoiceclient.h b/indra/newview/llvoiceclient.h
index d53f512d82..788ea3b3b3 100644
--- a/indra/newview/llvoiceclient.h
+++ b/indra/newview/llvoiceclient.h
@@ -438,7 +438,7 @@ public:
     bool getUserPTTState();
     void toggleUserPTTState(void);
     void inputUserControlState(bool down);  // interpret any sort of up-down mic-open control input according to ptt-toggle prefs
-    void setVoiceEnabled(bool enabled);
+    static void setVoiceEnabled(bool enabled);
 
     void setUsePTT(bool usePTT);
     void setPTTIsToggle(bool PTTIsToggle);
@@ -519,6 +519,7 @@ protected:
     LLPumpIO *m_servicePump;
 
     boost::signals2::connection  mSimulatorFeaturesReceivedSlot;
+    boost::signals2::connection  mRegionChangedCallbackSlot;
 
     LLCachedControl<bool> mVoiceEffectEnabled;
     LLCachedControl<std::string> mVoiceEffectDefault;
diff --git a/indra/newview/llvoicewebrtc.cpp b/indra/newview/llvoicewebrtc.cpp
index ad8f6927ed..b2e5de5371 100644
--- a/indra/newview/llvoicewebrtc.cpp
+++ b/indra/newview/llvoicewebrtc.cpp
@@ -258,6 +258,8 @@ void LLWebRTCVoiceClient::cleanupSingleton()
     }
     cleanUp();
     sessionState::clearSessions();
+
+    mStatusObservers.clear();
 }
 
 //---------------------------------------------------
@@ -403,8 +405,9 @@ void LLWebRTCVoiceClient::notifyStatusObservers(LLVoiceClientStatusObserver::ESt
     LL_DEBUGS("Voice") << "( " << LLVoiceClientStatusObserver::status2string(status) << " )"
                        << " mSession=" << mSession << LL_ENDL;
 
+    bool in_spatial_channel = inSpatialChannel();
     LL_DEBUGS("Voice") << " " << LLVoiceClientStatusObserver::status2string(status) << ", session channelInfo "
-                       << getAudioSessionChannelInfo() << ", proximal is " << inSpatialChannel() << LL_ENDL;
+                       << getAudioSessionChannelInfo() << ", proximal is " << in_spatial_channel << LL_ENDL;
 
     mIsProcessingChannels = status == LLVoiceClientStatusObserver::STATUS_JOINED;
 
@@ -412,7 +415,7 @@ void LLWebRTCVoiceClient::notifyStatusObservers(LLVoiceClientStatusObserver::ESt
     for (status_observer_set_t::iterator it = mStatusObservers.begin(); it != mStatusObservers.end();)
     {
         LLVoiceClientStatusObserver *observer = *it;
-        observer->onChange(status, channelInfo, inSpatialChannel());
+        observer->onChange(status, channelInfo, in_spatial_channel);
         // In case onError() deleted an entry.
         it = mStatusObservers.upper_bound(observer);
     }