Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pGeneratedNewStub determination #80128

Merged
merged 2 commits into from
Jan 5, 2023
Merged

Fix pGeneratedNewStub determination #80128

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64fa1c5
Fix pGeneratedNewStub determination
ChrisAhna Dec 30, 2022
4c8b8f0
Remove unnecessary ILStubCreatorHelperHolder concept
ChrisAhna Jan 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 47 additions & 40 deletions src/coreclr/vm/dllimport.cpp
View file
Open in desktop
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