Skip to content

Add LOCKREADIFFAILRET to unlocked MDInternalRW read methods #127958

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

Open
kevingosse wants to merge 4 commits into
base: main
Choose a base branch
from
+91 −33
Open

Add LOCKREADIFFAILRET to unlocked MDInternalRW read methods #127958

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e773849
Add LOCKREADIFFAILRET to unlocked MDInternalRW read methods
kevingosse May 8, 2026
69f50cf
Initialize out-params before lock acquisition
kevingosse May 8, 2026
3e9328f
Extend LOCKREADIFFAILRET to remaining unlocked MDInternalRW readers
kevingosse May 17, 2026
1c15491
Found a few more missing locks
noahfalk May 19, 2026
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
124 changes: 91 additions & 33 deletions src/coreclr/md/enc/mdinternalrw.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1566,10 +1566,13 @@ MDInternalRW::GetCustomAttributeProps( // S_OK or error.
mdToken *pTkType) // Put attribute type here.
{
HRESULT hr;
// Getting the custom value prop with a token, no need to lock!

_ASSERTE(TypeFromToken(at) == mdtCustomAttribute);

*pTkType = mdTokenNil;

LOCKREADIFFAILRET();
Comment thread
noahfalk marked this conversation as resolved.

// Do a linear search on compressed version as we do not want to
// depend on ICR.
//
Expand All @@ -1591,10 +1594,14 @@ MDInternalRW::GetCustomAttributeAsBlob(
void const **ppBlob, // [OUT] return the pointer to internal blob
ULONG *pcbSize) // [OUT] return the size of the blob
{
// Getting the custom value prop with a token, no need to lock!
HRESULT hr;
_ASSERTE(ppBlob && pcbSize && TypeFromToken(cv) == mdtCustomAttribute);

*ppBlob = NULL;
*pcbSize = 0;

LOCKREADIFFAILRET();

CustomAttributeRec *pCustomAttributeRec;

IfFailRet(m_pStgdb->m_MiniMd.GetCustomAttributeRecord(RidFromToken(cv), &pCustomAttributeRec));
Expand Down Expand Up @@ -1805,7 +1812,6 @@ MDInternalRW::GetNameOfTypeDef( // return hresult
LPCSTR* pszname, // pointer to an internal UTF8 string
LPCSTR* psznamespace) // pointer to the namespace.
{
// No need to lock this method.
HRESULT hr;

if (pszname != NULL)
Expand All @@ -1819,6 +1825,8 @@ MDInternalRW::GetNameOfTypeDef( // return hresult

if (TypeFromToken(classdef) == mdtTypeDef)
{
LOCKREADIFFAILRET();

TypeDefRec *pTypeDefRec;
IfFailRet(m_pStgdb->m_MiniMd.GetTypeDefRecord(RidFromToken(classdef), &pTypeDefRec));

Expand Down Expand Up @@ -1898,10 +1906,12 @@ MDInternalRW::GetNameOfMethodDef(
mdMethodDef md,
LPCSTR *pszMethodName)
{
// name of method will not change. So no need to lock
HRESULT hr;
MethodRec *pMethodRec;
*pszMethodName = NULL;

LOCKREADIFFAILRET();

MethodRec *pMethodRec;
IfFailRet(m_pStgdb->m_MiniMd.GetMethodRecord(RidFromToken(md), &pMethodRec));
IfFailRet(m_pStgdb->m_MiniMd.getNameOfMethod(pMethodRec, pszMethodName));
return S_OK;
Expand All @@ -1920,20 +1930,23 @@ MDInternalRW::GetNameAndSigOfMethodDef(
LPCSTR *pszMethodName)
{
HRESULT hr;
// we don't need lock here because name and signature will not change

// Output parameter should not be NULL
_ASSERTE(ppvSigBlob && pcbSigBlob);
_ASSERTE(TypeFromToken(methoddef) == mdtMethodDef);

MethodRec *pMethodRec;
*pszMethodName = NULL;
*ppvSigBlob = NULL;
*ppvSigBlob = NULL;
*pcbSigBlob = 0;

LOCKREADIFFAILRET();

MethodRec *pMethodRec;
IfFailRet(m_pStgdb->m_MiniMd.GetMethodRecord(RidFromToken(methoddef), &pMethodRec));
IfFailRet(m_pStgdb->m_MiniMd.getSignatureOfMethod(pMethodRec, ppvSigBlob, pcbSigBlob));
IfFailRet(m_pStgdb->m_MiniMd.getNameOfMethod(pMethodRec, pszMethodName));

return GetNameOfMethodDef(methoddef, pszMethodName);
return S_OK;
} // MDInternalRW::GetNameAndSigOfMethodDef


Expand All @@ -1946,11 +1959,12 @@ MDInternalRW::GetNameOfFieldDef( // return hresult
mdFieldDef fd, // given field
LPCSTR *pszFieldName)
{
// we don't need lock here because name of field will not change
HRESULT hr;
*pszFieldName = NULL;

LOCKREADIFFAILRET();

FieldRec *pFieldRec;
*pszFieldName = NULL;
IfFailRet(m_pStgdb->m_MiniMd.GetFieldRecord(RidFromToken(fd), &pFieldRec));
IfFailRet(m_pStgdb->m_MiniMd.getNameOfField(pFieldRec, pszFieldName));
return S_OK;
Expand All @@ -1973,7 +1987,7 @@ MDInternalRW::GetNameOfTypeRef( // return TypeDef's name
*psznamespace = NULL;
*pszname = NULL;

// we don't need lock here because name of a typeref will not change
LOCKREADIFFAILRET();

TypeRefRec *pTypeRefRec;
IfFailRet(m_pStgdb->m_MiniMd.GetTypeRefRecord(RidFromToken(classref), &pTypeRefRec));
Expand Down Expand Up @@ -2196,6 +2210,8 @@ MDInternalRW::GetCountNestedClasses( // return count of Nested classes.

*pcNestedClassesCount = 0;

LOCKREADIFFAILRET();

ulCount = m_pStgdb->m_MiniMd.getCountNestedClasss();

for (ULONG i = 1; i <= ulCount; i++)
Expand Down Expand Up @@ -2229,6 +2245,8 @@ MDInternalRW::GetNestedClasses( // Return actual count.

*pcNestedClasses = 0;

LOCKREADIFFAILRET();

ulCount = m_pStgdb->m_MiniMd.getCountNestedClasss();

for (ULONG i = 1; i <= ulCount; i++)
Expand Down Expand Up @@ -2285,11 +2303,12 @@ MDInternalRW::GetSigOfMethodDef(
_ASSERTE(TypeFromToken(methoddef) == mdtMethodDef);

HRESULT hr;
// We don't change MethodDef signature. No need to lock.

MethodRec *pMethodRec;
*ppSig = NULL;
*pcbSigBlob = 0;

LOCKREADIFFAILRET();

MethodRec *pMethodRec;
IfFailRet(m_pStgdb->m_MiniMd.GetMethodRecord(RidFromToken(methoddef), &pMethodRec));
IfFailRet(m_pStgdb->m_MiniMd.getSignatureOfMethod(pMethodRec, ppSig, pcbSigBlob));
return S_OK;
Expand All @@ -2310,11 +2329,12 @@ MDInternalRW::GetSigOfFieldDef(
_ASSERTE(TypeFromToken(fielddef) == mdtFieldDef);

HRESULT hr;
// We don't change Field's signature. No need to lock.

FieldRec *pFieldRec;
*ppSig = NULL;
*pcbSigBlob = 0;

LOCKREADIFFAILRET();

FieldRec *pFieldRec;
IfFailRet(m_pStgdb->m_MiniMd.GetFieldRecord(RidFromToken(fielddef), &pFieldRec));
IfFailRet(m_pStgdb->m_MiniMd.getSignatureOfField(pFieldRec, ppSig, pcbSigBlob));
return S_OK;
Expand All @@ -2332,34 +2352,35 @@ MDInternalRW::GetSigFromToken(
PCCOR_SIGNATURE * ppSig)
{
HRESULT hr;
Comment thread
noahfalk marked this conversation as resolved.
Comment thread
noahfalk marked this conversation as resolved.
// We don't change token's signature. Thus no need to lock.

*ppSig = NULL;
*pcbSig = 0;
switch (TypeFromToken(tk))
{
case mdtSignature:
{
LOCKREADIFFAILRET();
StandAloneSigRec *pRec;
IfFailGo(m_pStgdb->m_MiniMd.GetStandAloneSigRecord(RidFromToken(tk), &pRec));
IfFailGo(m_pStgdb->m_MiniMd.getSignatureOfStandAloneSig(pRec, ppSig, pcbSig));
IfFailRet(m_pStgdb->m_MiniMd.GetStandAloneSigRecord(RidFromToken(tk), &pRec));
IfFailRet(m_pStgdb->m_MiniMd.getSignatureOfStandAloneSig(pRec, ppSig, pcbSig));
return S_OK;
}
case mdtTypeSpec:
{
LOCKREADIFFAILRET();
TypeSpecRec *pRec;
IfFailGo(m_pStgdb->m_MiniMd.GetTypeSpecRecord(RidFromToken(tk), &pRec));
IfFailGo(m_pStgdb->m_MiniMd.getSignatureOfTypeSpec(pRec, ppSig, pcbSig));
IfFailRet(m_pStgdb->m_MiniMd.GetTypeSpecRecord(RidFromToken(tk), &pRec));
IfFailRet(m_pStgdb->m_MiniMd.getSignatureOfTypeSpec(pRec, ppSig, pcbSig));
return S_OK;
}
case mdtMethodDef:
{
IfFailGo(GetSigOfMethodDef(tk, pcbSig, ppSig));
IfFailRet(GetSigOfMethodDef(tk, pcbSig, ppSig));
return S_OK;
}
case mdtFieldDef:
{
IfFailGo(GetSigOfFieldDef(tk, pcbSig, ppSig));
IfFailRet(GetSigOfFieldDef(tk, pcbSig, ppSig));
return S_OK;
}
}
Expand All @@ -2370,10 +2391,7 @@ MDInternalRW::GetSigFromToken(
_ASSERTE(!"Unexpected token type");
#endif
*pcbSig = 0;
hr = META_E_INVALID_TOKEN_TYPE;

ErrExit:
return hr;
return META_E_INVALID_TOKEN_TYPE;
Comment thread
noahfalk marked this conversation as resolved.
} // MDInternalRW::GetSigFromToken


Expand Down Expand Up @@ -2584,12 +2602,13 @@ MDInternalRW::GetTypeOfInterfaceImpl( // return hresult
mdToken *ptkType)
{
HRESULT hr;
// no need to lock this function.

_ASSERTE(TypeFromToken(iiImpl) == mdtInterfaceImpl);

*ptkType = mdTypeDefNil;

LOCKREADIFFAILRET();

InterfaceImplRec *pIIRec;
IfFailRet(m_pStgdb->m_MiniMd.GetInterfaceImplRecord(RidFromToken(iiImpl), &pIIRec));
*ptkType = m_pStgdb->m_MiniMd.getInterfaceOfInterfaceImpl(pIIRec);
Expand All @@ -2611,6 +2630,15 @@ HRESULT MDInternalRW::GetMethodSpecProps( // S_OK or error.

_ASSERTE(TypeFromToken(mi) == mdtMethodSpec);

if (tkParent)
*tkParent = mdTokenNil;
if (ppvSigBlob)
*ppvSigBlob = NULL;
if (pcbSigBlob)
*pcbSigBlob = 0;

LOCKREADIFFAILRET();

IfFailGo(m_pStgdb->m_MiniMd.GetMethodSpecRecord(RidFromToken(mi), &pMethodSpecRec));

if (tkParent)
Expand Down Expand Up @@ -2670,24 +2698,26 @@ MDInternalRW::GetNameAndSigOfMemberRef( // meberref's name
{
HRESULT hr;
Comment thread
noahfalk marked this conversation as resolved.

// MemberRef's name and sig won't change. Don't need to lock this.

_ASSERTE(TypeFromToken(memberref) == mdtMemberRef);

MemberRefRec *pMemberRefRec;
*pszMemberRefName = NULL;
if (ppvSigBlob != NULL)
{
_ASSERTE(pcbSigBlob != NULL);
*ppvSigBlob = NULL;
*pcbSigBlob = 0;
}

LOCKREADIFFAILRET();

MemberRefRec *pMemberRefRec;
IfFailRet(m_pStgdb->m_MiniMd.GetMemberRefRecord(RidFromToken(memberref), &pMemberRefRec));
if (ppvSigBlob != NULL)
{
IfFailRet(m_pStgdb->m_MiniMd.getSignatureOfMemberRef(pMemberRefRec, ppvSigBlob, pcbSigBlob));
}
IfFailRet(m_pStgdb->m_MiniMd.getNameOfMemberRef(pMemberRefRec, pszMemberRefName));

return S_OK;
} // MDInternalRW::GetNameAndSigOfMemberRef

Expand Down Expand Up @@ -3243,6 +3273,19 @@ HRESULT MDInternalRW::GetGenericParamProps( // S_OK or error.
HRESULT hr = NOERROR;
GenericParamRec *pGenericParamRec = NULL;

if (pulSequence)
*pulSequence = 0;
if (pdwAttr)
*pdwAttr = 0;
if (ptOwner)
*ptOwner = mdTokenNil;
if (reserved)
*reserved = 0;
if (szName != NULL)
*szName = NULL;

LOCKREADIFFAILRET();

// See if this version of the metadata can do Generics
if (!m_pStgdb->m_MiniMd.SupportsGenerics())
IfFailGo(CLDB_E_INCOMPATIBLE);
Expand Down Expand Up @@ -3282,6 +3325,13 @@ HRESULT MDInternalRW::GetGenericParamConstraintProps( // S_OK or error.
GenericParamConstraintRec *pGPCRec;
RID ridRD = RidFromToken(rd);

if (ptGenericParam)
*ptGenericParam = mdGenericParamNil;
if (ptkConstraintType)
*ptkConstraintType = mdTokenNil;

LOCKREADIFFAILRET();

// See if this version of the metadata can do Generics
if (!m_pStgdb->m_MiniMd.SupportsGenerics())
IfFailGo(CLDB_E_INCOMPATIBLE);
Expand Down Expand Up @@ -3775,9 +3825,14 @@ HRESULT MDInternalRW::GetTypeSpecFromToken( // S_OK or error.
_ASSERTE(TypeFromToken(typespec) == mdtTypeSpec);
_ASSERTE(ppvSig && pcbSig);

*ppvSig = NULL;
*pcbSig = 0;

if (!IsValidToken(typespec))
return E_INVALIDARG;

LOCKREADIFFAILRET();

TypeSpecRec *pRec;
IfFailRet(m_pStgdb->m_MiniMd.GetTypeSpecRecord(RidFromToken(typespec), &pRec));

Expand Down Expand Up @@ -3969,6 +4024,9 @@ HRESULT MDInternalRW::EnumDeltaTokensInit( // return hresult
phEnum->m_EnumType = MDSimpleEnum;

HENUMInternal::InitDynamicArrayEnum(phEnum);

LOCKREADIFFAILRET();

for (index = 1; index <= m_pStgdb->m_MiniMd.m_Schema.m_cRecs[TBL_ENCLog]; ++index)
{
// Get the token type; see if it is a real token.
Expand Down
Loading