Fix pGeneratedNewStub determination (#80128)

* Fix pGeneratedNewStub determination

* Remove unnecessary ILStubCreatorHelperHolder concept

Author: ChrisAhna

src/coreclr/vm/dllimport.cpp

@@ -4745,11 +4745,25 @@ namespace
             RemoveILStubCacheEntry();
         }
 
+        inline bool CreatedTheAssociatedPublishedStubMD()
+        {
+            return m_bILStubCreator;
+        }
+
         inline void GetStubMethodDesc()
         {
             WRAPPER_NO_CONTRACT;
 
+            // The creator flag represents ownership of the associated stub MD and indicates that the
+            // stub MD has not been removed from the cache, so the lookup below is guaranteed to return
+            // this owned published stub MD.
+#ifdef _DEBUG
+            MethodDesc* pPreexistingStubMD = m_pStubMD;
+            bool createdThePreexistingMD = m_bILStubCreator;
+#endif // _DEBUG
+
             m_pStubMD = ::GetStubMethodDesc(m_pTargetMD, m_pParams, m_pHashParams, &m_amTracker, m_bILStubCreator, m_pStubMD);
+            _ASSERTE(!createdThePreexistingMD || (m_bILStubCreator && (m_pStubMD == pPreexistingStubMD)));
         }
 
         inline void RemoveILStubCacheEntry()
@@ -4776,20 +4790,6 @@ namespace
             m_amTracker.SuppressRelease();
         }
 
-        DEBUG_NOINLINE static void HolderEnter(ILStubCreatorHelper *pThis)
-        {
-            WRAPPER_NO_CONTRACT;
-            ANNOTATION_SPECIAL_HOLDER_CALLER_NEEDS_DYNAMIC_CONTRACT;
-            pThis->GetStubMethodDesc();
-        }
-
-        DEBUG_NOINLINE static void HolderLeave(ILStubCreatorHelper *pThis)
-        {
-            WRAPPER_NO_CONTRACT;
-            ANNOTATION_SPECIAL_HOLDER_CALLER_NEEDS_DYNAMIC_CONTRACT;
-            pThis->RemoveILStubCacheEntry();
-        }
-
     private:
         MethodDesc*                      m_pTargetMD;
         NDirectStubParameters*           m_pParams;
@@ -4800,8 +4800,6 @@ namespace
         bool                             m_bILStubCreator;     // Only the creator can remove the ILStub from the Cache
     };  //ILStubCreatorHelper
 
-    typedef Wrapper<ILStubCreatorHelper*, ILStubCreatorHelper::HolderEnter, ILStubCreatorHelper::HolderLeave> ILStubCreatorHelperHolder;
-
     MethodDesc* CreateInteropILStub(
                             ILStubState*             pss,
                             StubSigDesc*             pSigDesc,
@@ -4875,8 +4873,8 @@ namespace
                                 pSigDesc->m_pMT
                                 );
 
-        // The following two ILStubCreatorHelperHolder are to recover the status when an
-        // exception happen during the generation of the IL stubs. We need to free the
+        // The following ILStubCreatorHelper is to recover the status when an
+        // exception happens during the generation of the IL stubs. We need to free the
         // memory allocated and restore the ILStubCache.
         //
         // The following block is logically divided into two phases. The first phase is
@@ -4886,7 +4884,7 @@ namespace
         //
         // ilStubCreatorHelper contains an instance of AllocMemTracker which tracks the
         // allocated memory during the creation of MethodDesc so that we are able to remove
-        // them when releasing the ILStubCreatorHelperHolder or destructing ILStubCreatorHelper
+        // them when destructing ILStubCreatorHelper
 
         // When removing IL Stub from Cache, we have a constraint that only the thread which
         // creates the stub can remove it. Otherwise, any thread hits cache and gets the stub will
@@ -4899,10 +4897,8 @@ namespace
             ListLockHolder pILStubLock(pLoaderModule->GetDomain()->GetILStubGenLock());
 
             {
-                // The holder will free the allocated MethodDesc and restore the ILStubCache
-                // if exception happen.
-                ILStubCreatorHelperHolder pCreateOrGetStubHolder(&ilStubCreatorHelper);
-                pStubMD = pCreateOrGetStubHolder->GetStubMD();
+                ilStubCreatorHelper.GetStubMethodDesc();
+                pStubMD = ilStubCreatorHelper.GetStubMD();
 
                 ///////////////////////////////
                 //
@@ -4916,16 +4912,11 @@ namespace
 
                     ListLockEntryLockHolder pEntryLock(pEntry, FALSE);
 
-                    // We can release the holder for the first phase now
-                    pCreateOrGetStubHolder.SuppressRelease();
-
                     // We have the entry lock we need to use, so we can release the global lock.
                     pILStubLock.Release();
 
                     {
-                        // The holder will free the allocated MethodDesc and restore the ILStubCache
-                        // if exception happen. The reason to get the holder again is to
-                        ILStubCreatorHelperHolder pGenILHolder(&ilStubCreatorHelper);
+                        ilStubCreatorHelper.GetStubMethodDesc();
 
                         if (!pEntryLock.DeadlockAwareAcquire())
                         {
@@ -4950,11 +4941,11 @@ namespace
                             pILStubLock.Acquire();
 
                             // Assure that pStubMD we have now has not been destroyed by other threads
-                            pGenILHolder->GetStubMethodDesc();
+                            ilStubCreatorHelper.GetStubMethodDesc();
 
-                            while (pStubMD != pGenILHolder->GetStubMD())
+                            while (pStubMD != ilStubCreatorHelper.GetStubMD())
                             {
-                                pStubMD = pGenILHolder->GetStubMD();
+                                pStubMD = ilStubCreatorHelper.GetStubMD();
 
                                 pEntry.Assign(ListLockEntry::Find(pILStubLock, pStubMD, "il stub gen lock"));
                                 pEntryLock.Assign(pEntry, FALSE);
@@ -4980,7 +4971,7 @@ namespace
 
                                 pILStubLock.Acquire();
 
-                                pGenILHolder->GetStubMethodDesc();
+                                ilStubCreatorHelper.GetStubMethodDesc();
                             }
                         }
 
@@ -5064,22 +5055,38 @@ namespace
                                 sgh.SuppressRelease();
                             }
 
-                            if (pGeneratedNewStub)
-                            {
-                                *pGeneratedNewStub = true;
-                            }
-
                             pEntry->m_hrResultCode = S_OK;
                             break;
                         }
 
                         // Link the MethodDesc onto the method table with the lock taken
                         AddMethodDescChunkWithLockTaken(&params, pStubMD);
-
-                        pGenILHolder.SuppressRelease();
                     }
                 }
             }
+
+            // Callers use the new stub indicator to distinguish between 1) the case where a new stub
+            // MD was generated during this call and 2) the case where this function attached to a stub
+            // MD that was generated by some other call (either a call that completed earlier or a call
+            // on a racing thread). In particular, reliably detecting case (1) is crucial because it is
+            // the only case where this call permanently publishes a new stub MD into the cache,
+            // meaning it is the only case where the caller cannot safely free any allocations (such as
+            // a signature buffer) which the stub MD might reference.
+            //
+            // Set the indicator if and only if the stub MD that will be imminiently returned to the
+            // caller was created by the code above (and will therefore become a permanent member of
+            // the cache when the SuppressRelease occurs below). Note that, in the presence of racing
+            // threads, the current call may or may not have carried out IL generation for the stub;
+            // the only important thing is whether the current call was the one that created the stub
+            // MD earlier on.
+            if (ilStubCreatorHelper.CreatedTheAssociatedPublishedStubMD())
+            {
+                if (pGeneratedNewStub)
+                {
+                    *pGeneratedNewStub = true;
+                }
+            }
+
             ilStubCreatorHelper.SuppressRelease();
         }