Fix GetAppdomainStaticAddress for non-collectible assemblies (#41076)
authorDavid Mason <davmason@microsoft.com>
Mon, 24 Aug 2020 21:53:07 +0000 (14:53 -0700)
committerGitHub <noreply@github.com>
Mon, 24 Aug 2020 21:53:07 +0000 (14:53 -0700)
src/coreclr/src/vm/proftoeeinterfaceimpl.cpp
src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.cpp
src/tests/profiler/native/getappdomainstaticaddress/getappdomainstaticaddress.h
src/tests/profiler/native/metadatagetdispenser/metadatagetdispenser.cpp
src/tests/profiler/native/profilerstring.h
src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp
src/tests/profiler/native/rejitprofiler/rejitprofiler.h

index 487e123..2de646b 100644 (file)
@@ -3085,7 +3085,7 @@ HRESULT ProfToEEInterfaceImpl::GetRVAStaticAddress(ClassID classId,
  * Returns:
  *    S_OK on success,
  *    E_INVALIDARG if not an app domain static,
- *    CORPROF_E_DATAINCOMPLETE if not yet initialized.
+ *    CORPROF_E_DATAINCOMPLETE if not yet initialized or the module is being unloaded.
  *
  */
 HRESULT ProfToEEInterfaceImpl::GetAppDomainStaticAddress(ClassID classId,
@@ -3146,8 +3146,10 @@ HRESULT ProfToEEInterfaceImpl::GetAppDomainStaticAddress(ClassID classId,
         return CORPROF_E_DATAINCOMPLETE;
     }
 
-    if (typeHandle.GetModule()->GetLoaderAllocator() == NULL ||
-        typeHandle.GetModule()->GetLoaderAllocator()->GetExposedObject() == NULL)
+    // We might have caught a collectible assembly in the middle of being collected
+    Module *pModule = typeHandle.GetModule();
+    if (pModule->IsCollectible() &&
+        (pModule->GetLoaderAllocator() == NULL || pModule->GetLoaderAllocator()->GetExposedObject() == NULL))
     {
         return CORPROF_E_DATAINCOMPLETE;
     }
index f62a92a..70fcc60 100644 (file)
@@ -24,6 +24,8 @@ GetAppDomainStaticAddress::GetAppDomainStaticAddress() :
     refCount(0),
     failures(0),
     successes(0),
+    collectibleCount(0),
+    nonCollectibleCount(0),
     jitEventCount(0),
     gcTriggerThread(),
     gcWaitEvent(),
@@ -111,13 +113,14 @@ HRESULT GetAppDomainStaticAddress::Shutdown()
         this->pCorProfilerInfo = nullptr;
     }
 
-    if(failures == 0 && successes > 0)
+    if(failures == 0 && successes > 0 && collectibleCount > 0 && nonCollectibleCount > 0)
     {
         printf("PROFILER TEST PASSES\n");
     }
     else
     {
-        printf("Test failed number of failures=%d successes=%d\n", failures.load(), successes.load());
+        printf("Test failed number of failures=%d successes=%d collectibleCount=%d nonCollectibleCount=%d\n",
+            failures.load(), successes.load(), collectibleCount.load(), nonCollectibleCount.load());
     }
     fflush(stdout);
 
@@ -403,7 +406,7 @@ HRESULT GetAppDomainStaticAddress::GarbageCollectionFinished()
             // Module is being unloaded...
             continue;
         }
-        if (FAILED(hr))
+        else if (FAILED(hr))
         {
             printf("GetModuleMetaData returned 0x%x  for ModuleID 0x%" PRIxPTR "\n", hr, classModuleId);
             ++failures;
@@ -434,7 +437,7 @@ HRESULT GetAppDomainStaticAddress::GarbageCollectionFinished()
             // Class load not complete.  We can not inspect yet.
             continue;
         }
-        if (FAILED(hr))
+        else if (FAILED(hr))
         {
             printf("GetClassIDInfo2returned 0x%x\n", hr);
             ++failures;
@@ -515,6 +518,18 @@ HRESULT GetAppDomainStaticAddress::GarbageCollectionFinished()
                         ++failures;
                         continue;
                     }
+                    else if (hr != CORPROF_E_DATAINCOMPLETE)
+                    {
+                        String moduleName = GetModuleIDName(classModuleId);
+                        if (EndsWith(moduleName, WCHAR("unloadlibrary.dll")))
+                        {
+                            ++collectibleCount;
+                        }
+                        else
+                        {
+                            ++nonCollectibleCount;
+                        }
+                    }
                 }
             }
         }
index 009fb55..519700e 100644 (file)
@@ -88,6 +88,8 @@ private:
     std::atomic<int> refCount;
     std::atomic<ULONG32> failures;
     std::atomic<ULONG32> successes;
+    std::atomic<ULONG32> collectibleCount;
+    std::atomic<ULONG32> nonCollectibleCount;
 
     std::atomic<int> jitEventCount;
     std::thread gcTriggerThread;
index 93375b0..8abeb3e 100644 (file)
@@ -165,32 +165,6 @@ HRESULT MetaDataGetDispenser::GetDispenser(IMetaDataDispenserEx **disp)
 
 #else // WIN32
 
-bool EndsWith(const char *lhs, const char *rhs)
-{
-    size_t lhsLen = strlen(lhs);
-    size_t rhsLen = strlen(rhs);
-    if (lhsLen < rhsLen)
-    {
-        return false;
-    }
-
-    size_t lhsPos = lhsLen - rhsLen;
-    size_t rhsPos = 0;
-
-    while (rhsPos < rhsLen)
-    {
-        if (lhs[lhsPos] != rhs[rhsPos])
-        {
-            return false;
-        }
-
-        ++lhsPos;
-        ++rhsPos;
-    }
-
-    return true;
-}
-
 #ifdef __APPLE__
 string GetCoreCLRPath()
 {
index d7b6375..16ac3ec 100644 (file)
@@ -261,7 +261,7 @@ public:
         return temp;
     }
 
-    size_t Size() const
+    size_t Length() const
     {
         return wcslen(buffer);
     }
@@ -281,3 +281,55 @@ inline std::wostream& operator<<(std::wostream& os, const String& obj)
 
     return os;
 }
+
+inline bool EndsWith(const char *lhs, const char *rhs)
+{
+    size_t lhsLen = strlen(lhs);
+    size_t rhsLen = strlen(rhs);
+    if (lhsLen < rhsLen)
+    {
+        return false;
+    }
+
+    size_t lhsPos = lhsLen - rhsLen;
+    size_t rhsPos = 0;
+
+    while (rhsPos < rhsLen)
+    {
+        if (lhs[lhsPos] != rhs[rhsPos])
+        {
+            return false;
+        }
+
+        ++lhsPos;
+        ++rhsPos;
+    }
+
+    return true;
+}
+
+inline bool EndsWith(const String &lhs, const String &rhs)
+{
+    size_t lhsLength = lhs.Length();
+    size_t rhsLength = rhs.Length();
+    if (lhsLength < rhsLength)
+    {
+        return false;
+    }
+
+    size_t lhsPos = lhsLength - rhsLength;
+    size_t rhsPos = 0;
+
+    while (rhsPos < rhsLength)
+    {
+        if (std::tolower(lhs[lhsPos]) != std::tolower(rhs[rhsPos]))
+        {
+            return false;
+        }
+
+        ++lhsPos;
+        ++rhsPos;
+    }
+
+    return true;
+}
index 4da457c..298109b 100644 (file)
@@ -492,27 +492,3 @@ ModuleID ReJITProfiler::GetModuleIDForFunction(FunctionID functionId)
                                             typeArgs);
     return moduleId;
 }
-
-bool ReJITProfiler::EndsWith(const String &lhs, const String &rhs)
-{
-    if (lhs.Size() < rhs.Size())
-    {
-        return false;
-    }
-
-    size_t lhsPos = lhs.Size() - rhs.Size();
-    size_t rhsPos = 0;
-
-    while (rhsPos < rhs.Size())
-    {
-        if (std::tolower(lhs[lhsPos]) != std::tolower(rhs[rhsPos]))
-        {
-            return false;
-        }
-
-        ++lhsPos;
-        ++rhsPos;
-    }
-
-    return true;
-}
index 88f26f8..f105945 100644 (file)
@@ -51,7 +51,6 @@ private:
     FunctionID GetFunctionIDFromToken(ModuleID module, mdMethodDef token);
     mdMethodDef GetMethodDefForFunction(FunctionID functionId);
     ModuleID GetModuleIDForFunction(FunctionID functionId);
-    bool EndsWith(const String &lhs, const String &rhs);
 
     ICorProfilerInfo10 *_profInfo10;
     std::atomic<int> _failures;