Spurious loss of TLS object in case that a key is reused multiple times
lhmouse opened this issue · 2 comments
lhmouse commented
In src/env/thread_env.c
:
static TlsObject *GetTlsObject(TlsThread *pThread, TlsKey *pKey){
_MCFCRT_ASSERT(pThread);
if(!pKey){
return nullptr;
}
TlsObject *const pObject = (TlsObject *)_MCFCRT_AvlFind(&(pThread->avlObjects), (intptr_t)pKey, &ObjectComparatorNodeKey);
if(pObject && (pObject->uCounter != pKey->uCounter)){ // [1]
const _MCFCRT_TlsDestructor pfnDestructor = pObject->pfnDestructor;
if(pfnDestructor){
(*pfnDestructor)(pObject->nContext, pObject->abyStorage);
}
TlsObject *const pPrev = pObject->pPrevByThread;
TlsObject *const pNext = pObject->pNextByThread;
if(pPrev){
pPrev->pNextByThread = pNext;
}
if(pNext){
pNext->pPrevByThread = pPrev;
}
_MCFCRT_AvlDetach((_MCFCRT_AvlNodeHeader *)pObject);
_MCFCRT_free(pObject);
return nullptr;
}
return pObject;
}
Since a TLS key is the address of a block of memory, if a key is allocated after another one is freed, they could accidentally have the same value. I was aware of it and, as a result, TLS keys were assigned auto incremental IDs (the uCounter
member) in order to check for reused keys. This turns out to be insufficient, as the if
statement at [1] only does the check once. If a TLS key is reused twice or more, GetTlsObject()
might not return a valid pointer even if the TLS object actually resides in the map.
lhmouse commented
Fixed.
lhmouse commented
Well I should have kept thread_env.c
up-to-date with the one in MCFCRT since long ago. It seemed lost of sync somehow.