Skip to content

Commit

Permalink
[release/7.0] Fix pGeneratedNewStub determination (#80269)
Browse files Browse the repository at this point in the history 
* Fix pGeneratedNewStub determination

* Remove unnecessary ILStubCreatorHelperHolder concept

Co-authored-by: Chris Ahna <chrisahn@microsoft.com>
  • Loading branch information
@github-actions @ChrisAhna
github-actions[bot] and ChrisAhna authored Jan 9, 2023
1 parent baf7e5d commit 2f7c3f4
Showing 1 changed file with 47 additions and 40 deletions.
87 changes: 47 additions & 40 deletions src/coreclr/vm/dllimport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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();

///////////////////////////////
//
Expand All @@ -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())
{
Expand All @@ -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);
Expand All @@ -4980,7 +4971,7 @@ namespace

pILStubLock.Acquire();

pGenILHolder->GetStubMethodDesc();
ilStubCreatorHelper.GetStubMethodDesc();
}
}

Expand Down Expand Up @@ -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();
}

Expand Down

0 comments on commit 2f7c3f4

Please sign in to comment.