From e08a9c43001badaad5f35e911357fda9e27b8689 Mon Sep 17 00:00:00 2001
From: Andrey Kleshchev <andreykproductengine@lindenlab.com>
Date: Thu, 25 Nov 2021 00:03:37 +0200
Subject: [PATCH] SL-16412 More region coroutine crashes

LLViewerRegionImpl (mImpl) exists only so long as region does, and regions get cleaned before coroutines are cleaned.
---
 indra/newview/llviewerregion.cpp | 58 +++++++++++++++++---------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/indra/newview/llviewerregion.cpp b/indra/newview/llviewerregion.cpp
index 198fe1563cc..6d0eaed6186 100644
--- a/indra/newview/llviewerregion.cpp
+++ b/indra/newview/llviewerregion.cpp
@@ -242,9 +242,9 @@ class LLViewerRegionImpl
 	LLVector3   mLastCameraOrigin;
 	U32         mLastCameraUpdate;
 
-    void        requestBaseCapabilitiesCoro(U64 regionHandle);
-    void        requestBaseCapabilitiesCompleteCoro(U64 regionHandle);
-    void        requestSimulatorFeatureCoro(std::string url, U64 regionHandle);
+    static void        requestBaseCapabilitiesCoro(U64 regionHandle);
+    static void        requestBaseCapabilitiesCompleteCoro(U64 regionHandle);
+    static void        requestSimulatorFeatureCoro(std::string url, U64 regionHandle);
 };
 
 void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle)
@@ -272,6 +272,7 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle)
             LL_WARNS("AppInit", "Capabilities") << "Attempting to get capabilities for region that no longer exists!" << LL_ENDL;
             return; // this error condition is not recoverable.
         }
+        LLViewerRegionImpl* impl = regionp->getRegionImplNC();
         LL_DEBUGS("AppInit", "Capabilities") << "requesting seed caps for handle " << regionHandle 
                                              << " name " << regionp->getName() << LL_ENDL;
 
@@ -286,32 +287,33 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle)
         newRegionEntry(*regionp);
 
         // After a few attempts, continue login.  But keep trying to get the caps:
-        if (mSeedCapAttempts >= mSeedCapMaxAttemptsBeforeLogin &&
+        if (impl->mSeedCapAttempts >= impl->mSeedCapMaxAttemptsBeforeLogin &&
             STATE_SEED_GRANTED_WAIT == LLStartUp::getStartupState())
         {
             LLStartUp::setStartupState(STATE_SEED_CAP_GRANTED);
         }
 
-        if (mSeedCapAttempts > mSeedCapMaxAttempts)
+        if (impl->mSeedCapAttempts > impl->mSeedCapMaxAttempts)
         {
             // *TODO: Give a user pop-up about this error?
-            LL_WARNS("AppInit", "Capabilities") << "Failed to get seed capabilities from '" << url << "' after " << mSeedCapAttempts << " attempts.  Giving up!" << LL_ENDL;
+            LL_WARNS("AppInit", "Capabilities") << "Failed to get seed capabilities from '" << url << "' after " << impl->mSeedCapAttempts << " attempts.  Giving up!" << LL_ENDL;
             return;  // this error condition is not recoverable.
         }
 
-        S32 id = ++mHttpResponderID;
+        S32 id = ++(impl->mHttpResponderID);
 
         LLSD capabilityNames = LLSD::emptyArray();
-        buildCapabilityNames(capabilityNames);
+        impl->buildCapabilityNames(capabilityNames);
 
         LL_INFOS("AppInit", "Capabilities") << "Requesting seed from " << url 
                                             << " region name " << regionp->getName()
                                             << " region id " << regionp->getRegionID()
                                             << " handle " << regionp->getHandle()
-                                            << " (attempt #" << mSeedCapAttempts + 1 << ")" << LL_ENDL;
+                                            << " (attempt #" << impl->mSeedCapAttempts + 1 << ")" << LL_ENDL;
 		LL_DEBUGS("AppInit", "Capabilities") << "Capabilities requested: " << capabilityNames << LL_ENDL;
 
         regionp = NULL;
+        impl = NULL;
         result = httpAdapter->postAndSuspend(httpRequest, url, capabilityNames);
 
         if (STATE_WORLD_INIT > LLStartUp::getStartupState())
@@ -325,8 +327,6 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle)
             return;
         }
 
-        ++mSeedCapAttempts;
-
         regionp = LLWorld::getInstance()->getRegionFromHandle(regionHandle);
         if (!regionp) //region was removed
         {
@@ -334,7 +334,11 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle)
             return; // this error condition is not recoverable.
         }
 
-        if (id != mHttpResponderID) // region is no longer referring to this request
+        impl = regionp->getRegionImplNC();
+
+        ++impl->mSeedCapAttempts;
+
+        if (id != impl->mHttpResponderID) // region is no longer referring to this request
         {
             LL_WARNS("AppInit", "Capabilities") << "Received results for a stale capabilities request!" << LL_ENDL;
             // setup for retry.
@@ -391,7 +395,6 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle)
     {   // *HACK: we're waiting for the ServerReleaseNotes
         regionp->showReleaseNotes();
     }
-
 }
 
 
@@ -452,6 +455,7 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCompleteCoro(U64 regionHandle)
             LL_WARNS("AppInit", "Capabilities") << "Received capabilities for region that no longer exists!" << LL_ENDL;
             break; // this error condition is not recoverable.
         }
+        LLViewerRegionImpl* impl = regionp->getRegionImplNC();
 
         // remove the http_result from the llsd
         result.erase("http_result");
@@ -464,30 +468,30 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCompleteCoro(U64 regionHandle)
         }
 
 #if 0
-        log_capabilities(mCapabilities);
+        log_capabilities(impl->mCapabilities);
 #endif
 
-        if (mCapabilities.size() != mSecondCapabilitiesTracker.size())
+        if (impl->mCapabilities.size() != impl->mSecondCapabilitiesTracker.size())
         {
             LL_WARNS("AppInit", "Capabilities")
                 << "Sim sent duplicate base caps that differ in size from what we initially received - most likely content. "
-                << "mCapabilities == " << mCapabilities.size()
-                << " mSecondCapabilitiesTracker == " << mSecondCapabilitiesTracker.size()
+                << "mCapabilities == " << impl->mCapabilities.size()
+                << " mSecondCapabilitiesTracker == " << impl->mSecondCapabilitiesTracker.size()
                 << LL_ENDL;
 #ifdef DEBUG_CAPS_GRANTS
             LL_WARNS("AppInit", "Capabilities")
                 << "Initial Base capabilities: " << LL_ENDL;
 
-            log_capabilities(mCapabilities);
+            log_capabilities(impl->mCapabilities);
 
             LL_WARNS("AppInit", "Capabilities")
                 << "Latest base capabilities: " << LL_ENDL;
 
-            log_capabilities(mSecondCapabilitiesTracker);
+            log_capabilities(impl->mSecondCapabilitiesTracker);
 
 #endif
 
-            if (mSecondCapabilitiesTracker.size() > mCapabilities.size())
+            if (impl->mSecondCapabilitiesTracker.size() > impl->mCapabilities.size())
             {
                 // *HACK Since we were granted more base capabilities in this grant request than the initial, replace
                 // the old with the new. This shouldn't happen i.e. we should always get the same capabilities from a
@@ -495,19 +499,17 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCompleteCoro(U64 regionHandle)
                 // inventory api capability grants.
 
                 // Need to clear a std::map before copying into it because old keys take precedence.
-                mCapabilities.clear();
-                mCapabilities = mSecondCapabilitiesTracker;
+                impl->mCapabilities.clear();
+                impl->mCapabilities = impl->mSecondCapabilitiesTracker;
             }
         }
         else
         {
             LL_DEBUGS("CrossingCaps") << "Sim sent multiple base cap grants with matching sizes." << LL_ENDL;
         }
-        mSecondCapabilitiesTracker.clear();
+        impl->mSecondCapabilitiesTracker.clear();
     } 
     while (false);
-
-
 }
 
 void LLViewerRegionImpl::requestSimulatorFeatureCoro(std::string url, U64 regionHandle)
@@ -2242,7 +2244,7 @@ void LLViewerRegion::requestSimulatorFeatures()
     {
         std::string coroname =
             LLCoros::instance().launch("LLViewerRegionImpl::requestSimulatorFeatureCoro",
-                                       boost::bind(&LLViewerRegionImpl::requestSimulatorFeatureCoro, mImpl, url, getHandle()));
+                                       boost::bind(&LLViewerRegionImpl::requestSimulatorFeatureCoro, url, getHandle()));
         
         LL_INFOS("AppInit", "SimulatorFeatures") << "Launching " << coroname << " requesting simulator features from " << url << " for region " << getRegionID() << LL_ENDL;
     }
@@ -3058,7 +3060,7 @@ void LLViewerRegion::setSeedCapability(const std::string& url)
 		//to the "original" seed cap received and determine why there is problem!
         std::string coroname =
             LLCoros::instance().launch("LLEnvironmentRequest::requestBaseCapabilitiesCompleteCoro",
-            boost::bind(&LLViewerRegionImpl::requestBaseCapabilitiesCompleteCoro, mImpl, getHandle()));
+            boost::bind(&LLViewerRegionImpl::requestBaseCapabilitiesCompleteCoro, getHandle()));
 		return;
     }
 	
@@ -3070,7 +3072,7 @@ void LLViewerRegion::setSeedCapability(const std::string& url)
 
     std::string coroname =
         LLCoros::instance().launch("LLViewerRegionImpl::requestBaseCapabilitiesCoro",
-        boost::bind(&LLViewerRegionImpl::requestBaseCapabilitiesCoro, mImpl, getHandle()));
+        boost::bind(&LLViewerRegionImpl::requestBaseCapabilitiesCoro, getHandle()));
 
     LL_INFOS("AppInit", "Capabilities") << "Launching " << coroname << " requesting seed capabilities from " << url << " for region " << getRegionID() << LL_ENDL;
 }
-- 
GitLab