Reapply "Replace CordbProcess::GetSharedDomain with GetAppDomain" (#117224)

Fix on top of the original: use Assembly as the ID for CordbAssembly instead of DomainAssembly.

Author: elinor-fung

src/coreclr/debug/daccess/dacdbiimpl.cpp

@@ -4856,14 +4856,12 @@ VMPTR_OBJECTHANDLE DacDbiInterfaceImpl::GetCurrentCustomDebuggerNotification(VMP
     return vmObjHandle;
 }
 
-// Return the current appdomain the specified thread is in.
-VMPTR_AppDomain DacDbiInterfaceImpl::GetCurrentAppDomain(VMPTR_Thread vmThread)
+// Return the current appdomain.
+VMPTR_AppDomain DacDbiInterfaceImpl::GetCurrentAppDomain()
 {
     DD_ENTER_MAY_THROW;
 
-    Thread *    pThread    = vmThread.GetDacPtr();
     AppDomain * pAppDomain = AppDomain::GetCurrentDomain();
-
     VMPTR_AppDomain vmAppDomain = VMPTR_AppDomain::NullPtr();
     vmAppDomain.SetDacTargetPtr(PTR_HOST_TO_TADDR(pAppDomain));
     return vmAppDomain;

src/coreclr/debug/daccess/dacdbiimpl.h

@@ -822,9 +822,8 @@ class DacDbiInterfaceImpl :
     // (or a dump was generated while in this callback)
     VMPTR_OBJECTHANDLE GetCurrentCustomDebuggerNotification(VMPTR_Thread vmThread);
 
-
-    // Return the current appdomain the specified thread is in.
-    VMPTR_AppDomain GetCurrentAppDomain(VMPTR_Thread vmThread);
+    // Return the current appdomain
+    VMPTR_AppDomain GetCurrentAppDomain();
 
     // Given an assembly ref token and metadata scope (via the DomainAssembly), resolve the assembly.
     VMPTR_DomainAssembly ResolveAssembly(VMPTR_DomainAssembly vmScope, mdToken tkAssemblyRef);

src/coreclr/debug/di/module.cpp

@@ -65,12 +65,12 @@ CordbModule::CordbModule(
         pProcess->GetDAC()->GetDomainAssemblyData(vmDomainAssembly, &dfInfo); // throws
 
         m_pAppDomain = pProcess->LookupOrCreateAppDomain(dfInfo.vmAppDomain);
+        _ASSERTE(m_pAppDomain == pProcess->GetAppDomain());
         m_pAssembly  = m_pAppDomain->LookupOrCreateAssembly(dfInfo.vmDomainAssembly);
     }
     else
     {
-        // Not yet implemented
-        m_pAppDomain = pProcess->GetSharedAppDomain();
+        m_pAppDomain = pProcess->GetAppDomain();
         m_pAssembly = m_pAppDomain->LookupOrCreateAssembly(modInfo.vmAssembly);
     }
 #ifdef _DEBUG

src/coreclr/debug/di/process.cpp

@@ -921,7 +921,6 @@ CordbProcess::CordbProcess(ULONG64 clrInstanceId,
     m_unmanagedThreads(11),
 #endif
     m_appDomains(11),
-    m_sharedAppDomain(0),
     m_steppers(11),
     m_continueCounter(1),
     m_flushCounter(0),
@@ -956,7 +955,6 @@ CordbProcess::CordbProcess(ULONG64 clrInstanceId,
     m_iFirstPatch(0),
     m_hHelperThread(NULL),
     m_dispatchedEvent(DB_IPCE_DEBUGGER_INVALID),
-    m_pDefaultAppDomain(NULL),
     m_hDacModule(hDacModule),
     m_pDacPrimitives(NULL),
     m_pEventChannel(NULL),
@@ -1059,8 +1057,6 @@ CordbProcess::~CordbProcess()
     // We shouldn't still be in Cordb's list of processes. Unfortunately, our root Cordb object
     // may have already been deleted b/c we're at the mercy of ref-counting, so we can't check.
 
-	_ASSERTE(m_sharedAppDomain == NULL);
-
     m_processMutex.Destroy();
     m_StopGoLock.Destroy();
 
@@ -1288,16 +1284,8 @@ void CordbProcess::NeuterChildren()
 
     m_userThreads.NeuterAndClear(GetProcessLock());
 
-    m_pDefaultAppDomain = NULL;
-
     // Frees per-appdomain left-side resources. See assumptions above.
     m_appDomains.NeuterAndClear(GetProcessLock());
-    if (m_sharedAppDomain != NULL)
-    {
-        m_sharedAppDomain->Neuter();
-        m_sharedAppDomain->InternalRelease();
-        m_sharedAppDomain = NULL;
-    }
 
     m_steppers.NeuterAndClear(GetProcessLock());
 
@@ -2306,10 +2294,10 @@ HRESULT CordbProcess::EnumerateHeapRegions(ICorDebugHeapSegmentEnum **ppRegions)
 
 HRESULT CordbProcess::GetObject(CORDB_ADDRESS addr, ICorDebugObjectValue **ppObject)
 {
-    return this->GetObjectInternal(addr, nullptr, ppObject);
+    return this->GetObjectInternal(addr, ppObject);
 }
 
-HRESULT CordbProcess::GetObjectInternal(CORDB_ADDRESS addr, CordbAppDomain* pAppDomainOverride, ICorDebugObjectValue **pObject)
+HRESULT CordbProcess::GetObjectInternal(CORDB_ADDRESS addr, ICorDebugObjectValue **pObject)
 {
     HRESULT hr = S_OK;
 
@@ -2333,7 +2321,7 @@ HRESULT CordbProcess::GetObjectInternal(CORDB_ADDRESS addr, CordbAppDomain* pApp
 
             CordbAppDomain *cdbAppDomain = NULL;
             CordbType *pType = NULL;
-            hr = GetTypeForObject(addr, pAppDomainOverride, &pType, &cdbAppDomain);
+            hr = GetTypeForObject(addr, &pType, &cdbAppDomain);
 
             if (SUCCEEDED(hr))
             {
@@ -2441,7 +2429,7 @@ HRESULT CordbProcess::GetTypeForTypeID(COR_TYPEID id, ICorDebugType **ppType)
         GetDAC()->GetObjectExpandedTypeInfoFromID(AllBoxed, VMPTR_AppDomain::NullPtr(), id, &data);
 
         CordbType *type = 0;
-        hr = CordbType::TypeDataToType(GetSharedAppDomain(), &data, &type);
+        hr = CordbType::TypeDataToType(GetAppDomain(), &data, &type);
 
         if (SUCCEEDED(hr))
             hr = type->QueryInterface(IID_ICorDebugType, (void**)ppType);
@@ -2569,7 +2557,7 @@ COM_METHOD CordbProcess::EnumerateLoaderHeapMemoryRegions(ICorDebugMemoryRangeEn
     return hr;
 }
 
-HRESULT CordbProcess::GetTypeForObject(CORDB_ADDRESS addr, CordbAppDomain* pAppDomainOverride, CordbType **ppType, CordbAppDomain **pAppDomain)
+HRESULT CordbProcess::GetTypeForObject(CORDB_ADDRESS addr, CordbType **ppType, CordbAppDomain **pAppDomain)
 {
     VMPTR_AppDomain appDomain;
     VMPTR_Module mod;
@@ -2578,11 +2566,7 @@ HRESULT CordbProcess::GetTypeForObject(CORDB_ADDRESS addr, CordbAppDomain* pAppD
     HRESULT hr = E_FAIL;
     if (GetDAC()->GetAppDomainForObject(addr, &appDomain, &mod, &domainAssembly))
     {
-        if (pAppDomainOverride)
-        {
-            appDomain = pAppDomainOverride->GetADToken();
-        }
-        CordbAppDomain *cdbAppDomain = appDomain.IsNull() ? GetSharedAppDomain() : LookupOrCreateAppDomain(appDomain);
+        CordbAppDomain *cdbAppDomain = appDomain.IsNull() ? GetAppDomain() : LookupOrCreateAppDomain(appDomain);
 
         _ASSERTE(cdbAppDomain);
 
@@ -5369,19 +5353,6 @@ void CordbProcess::RawDispatchEvent(
             }
             _ASSERTE (pAppDomain != NULL);
 
-            // See if this is the default AppDomain exiting.  This should only happen very late in
-            // the shutdown cycle, and so we shouldn't do anything significant with m_pDefaultDomain==NULL.
-            // We should try and remove m_pDefaultDomain entirely since we can't count on it always existing.
-            if (pAppDomain == m_pDefaultAppDomain)
-            {
-                m_pDefaultAppDomain = NULL;
-            }
-
-            // Update any threads which were last seen in this AppDomain.  We don't
-            // get any notification when a thread leaves an AppDomain, so our idea
-            // of what AppDomain the thread is in may be out of date.
-            UpdateThreadsForAdUnload( pAppDomain );
-
             // This will still maintain weak references so we could call Continue.
             AddToNeuterOnContinueList(pAppDomain);
 
@@ -8717,19 +8688,23 @@ CordbAppDomain * CordbProcess::LookupOrCreateAppDomain(VMPTR_AppDomain vmAppDoma
     return CacheAppDomain(vmAppDomain);
 }
 
-CordbAppDomain * CordbProcess::GetSharedAppDomain()
+CordbAppDomain * CordbProcess::GetAppDomain()
 {
-    if (m_sharedAppDomain == NULL)
+    // Return the one and only app domain
+    HASHFIND find;
+    CordbAppDomain* appDomain = m_appDomains.FindFirst(&find);
+    if (appDomain != NULL)
     {
-        CordbAppDomain *pAD = new CordbAppDomain(this, VMPTR_AppDomain::NullPtr());
-        if (InterlockedCompareExchangeT<CordbAppDomain*>(&m_sharedAppDomain, pAD, NULL) != NULL)
-        {
-            delete pAD;
-        }
-		m_sharedAppDomain->InternalAddRef();
+        const ULONG appDomainId = 1; // DefaultADID in appdomain.hpp
+        ULONG32 id;
+        HRESULT hr = appDomain->GetID(&id);
+        TargetConsistencyCheck(SUCCEEDED(hr) && id == appDomainId);
+        return appDomain;
     }
 
-    return m_sharedAppDomain;
+    VMPTR_AppDomain vmAppDomain = GetDAC()->GetCurrentAppDomain();
+    appDomain = LookupOrCreateAppDomain(vmAppDomain);
+    return appDomain;
 }
 
 //---------------------------------------------------------------------------------------
@@ -8765,10 +8740,6 @@ CordbAppDomain * CordbProcess::CacheAppDomain(VMPTR_AppDomain vmAppDomain)
     // The cache will take ownership.
     m_appDomains.AddBaseOrThrow(pAppDomain);
 
-    // If this assert fires, then it likely means the target is corrupted.
-    TargetConsistencyCheck(m_pDefaultAppDomain == NULL);
-    m_pDefaultAppDomain = pAppDomain;
-
     CordbAppDomain * pReturn = pAppDomain;
     pAppDomain.ClearAndMarkDontNeuter();
 
@@ -15263,46 +15234,6 @@ HRESULT CordbProcess::IsReadyForDetach()
     return S_OK;
 }
 
-
-/*
- * Look for any thread which was last seen in the specified AppDomain.
- * The CordbAppDomain object is about to be neutered due to an AD Unload
- * So the thread must no longer be considered to be in that domain.
- * Note that this is a workaround due to the existence of the (possibly incorrect)
- * cached AppDomain value.  Ideally we would remove the cached value entirely
- * and there would be no need for this.
- *
- * @dbgtodo: , appdomain: We should remove CordbThread::m_pAppDomain in the V3 architecture.
- * If we need the thread's current domain, we should get it accurately with DAC.
- */
-void CordbProcess::UpdateThreadsForAdUnload(CordbAppDomain * pAppDomain)
-{
-    INTERNAL_API_ENTRY(this);
-
-    // If we're doing an AD unload then we should have already seen the ATTACH
-    // notification for the default domain.
-    //_ASSERTE( m_pDefaultAppDomain != NULL );
-    // @dbgtodo appdomain: fix Default domain invariants with DAC-izing Appdomain work.
-
-    RSLockHolder lockHolder(GetProcessLock());
-
-    CordbThread* t;
-    HASHFIND find;
-
-    // We don't need to prepopulate here (to collect LS state) because we're just updating RS state.
-    for (t =  m_userThreads.FindFirst(&find);
-         t != NULL;
-         t =  m_userThreads.FindNext(&find))
-    {
-        if( t->GetAppDomain() == pAppDomain )
-        {
-            // This thread cannot actually be in this AppDomain anymore (since it's being
-            // unloaded).  Reset it to point to the default AppDomain
-            t->m_pAppDomain = m_pDefaultAppDomain;
-        }
-    }
-}
-
 // CordbProcess::LookupClass
 // Looks up a previously constructed CordbClass instance without creating. May return NULL if the
 // CordbClass instance doesn't exist.

src/coreclr/debug/di/rsappdomain.cpp

@@ -388,7 +388,7 @@ void CordbAppDomain::AssemblyEnumerationCallback(VMPTR_DomainAssembly vmDomainAs
 // Cache a new assembly
 //
 // Arguments:
-//      vmDomainAssembly - new assembly to add to cache
+//      vmAssembly - new assembly to add to cache
 //
 // Return Value:
 //    Pointer to Assembly in cache.
@@ -398,29 +398,15 @@ void CordbAppDomain::AssemblyEnumerationCallback(VMPTR_DomainAssembly vmDomainAs
 //    Caller guarantees assembly is not already added.
 //    Called under the stop-go lock.
 //
-// Notes:
-//
-CordbAssembly * CordbAppDomain::CacheAssembly(VMPTR_DomainAssembly vmDomainAssembly)
+CordbAssembly * CordbAppDomain::CacheAssembly(VMPTR_Assembly vmAssembly, VMPTR_DomainAssembly vmDomainAssembly)
 {
     INTERNAL_API_ENTRY(GetProcess());
 
-    VMPTR_Assembly vmAssembly;
-    GetProcess()->GetDAC()->GetAssemblyFromDomainAssembly(vmDomainAssembly, &vmAssembly);
-
     RSInitHolder<CordbAssembly> pAssembly(new CordbAssembly(this, vmAssembly, vmDomainAssembly));
 
     return pAssembly.TransferOwnershipToHash(&m_assemblies);
 }
 
-CordbAssembly * CordbAppDomain::CacheAssembly(VMPTR_Assembly vmAssembly)
-{
-    INTERNAL_API_ENTRY(GetProcess());
-
-    RSInitHolder<CordbAssembly> pAssembly(new CordbAssembly(this, vmAssembly, VMPTR_DomainAssembly()));
-
-    return pAssembly.TransferOwnershipToHash(&m_assemblies);
-}
-
 //---------------------------------------------------------------------------------------
 //
 // Build up cache of assmeblies
@@ -785,7 +771,9 @@ void CordbAppDomain::RemoveAssemblyFromCache(VMPTR_DomainAssembly vmDomainAssemb
 {
     // This will handle if the assembly is not in the hash.
     // This could happen if we attach right before an assembly-unload event.
-    m_assemblies.RemoveBase(VmPtrToCookie(vmDomainAssembly));
+    VMPTR_Assembly vmAssembly;
+    GetProcess()->GetDAC()->GetAssemblyFromDomainAssembly(vmDomainAssembly, &vmAssembly);
+    m_assemblies.RemoveBase(VmPtrToCookie(vmAssembly));
 }
 
 //---------------------------------------------------------------------------------------
@@ -801,15 +789,16 @@ void CordbAppDomain::RemoveAssemblyFromCache(VMPTR_DomainAssembly vmDomainAssemb
 //
 CordbAssembly * CordbAppDomain::LookupOrCreateAssembly(VMPTR_DomainAssembly vmDomainAssembly)
 {
-    CordbAssembly * pAssembly = m_assemblies.GetBase(VmPtrToCookie(vmDomainAssembly));
+    VMPTR_Assembly vmAssembly;
+    GetProcess()->GetDAC()->GetAssemblyFromDomainAssembly(vmDomainAssembly, &vmAssembly);
+    CordbAssembly * pAssembly = m_assemblies.GetBase(VmPtrToCookie(vmAssembly));
     if (pAssembly != NULL)
     {
         return pAssembly;
     }
-    return CacheAssembly(vmDomainAssembly);
+    return CacheAssembly(vmAssembly, vmDomainAssembly);
 }
 
-
 //
 CordbAssembly * CordbAppDomain::LookupOrCreateAssembly(VMPTR_Assembly vmAssembly)
 {
@@ -818,7 +807,7 @@ CordbAssembly * CordbAppDomain::LookupOrCreateAssembly(VMPTR_Assembly vmAssembly
     {
         return pAssembly;
     }
-    return CacheAssembly(vmAssembly);
+    return CacheAssembly(vmAssembly, VMPTR_DomainAssembly());
 }
 
 

src/coreclr/debug/di/rsassembly.cpp

@@ -29,7 +29,7 @@ CordbAssembly::CordbAssembly(CordbAppDomain *       pAppDomain,
                              VMPTR_DomainAssembly   vmDomainAssembly)
 
     : CordbBase(pAppDomain->GetProcess(),
-                vmDomainAssembly.IsNull() ? VmPtrToCookie(vmAssembly) : VmPtrToCookie(vmDomainAssembly),
+                VmPtrToCookie(vmAssembly),
                 enumCordbAssembly),
       m_vmAssembly(vmAssembly),
       m_vmDomainAssembly(vmDomainAssembly),