physfs icon indicating copy to clipboard operation
physfs copied to clipboard

Thread Sanitizer flags `__PHYSFS_platformGrabMutex` as having a data race

Open nicebyte opened this issue 1 year ago • 1 comments

It appears that TSan gets tripped up by reading/writing the owner of the mutex in the function below:

int __PHYSFS_platformGrabMutex(void *mutex)
{
    PthreadMutex *m = (PthreadMutex *) mutex;
    pthread_t tid = pthread_self();
    if (m->owner != tid)
    {
        if (pthread_mutex_lock(&m->mutex) != 0)
            return 0;
        m->owner = tid;
    } /* if */

    m->count++;
    return 1;
} /* __PHYSFS_platformGrabMutex */

I've added this function to my suppressions file, but would be nice to do something about it? Technically, an atomic load/store is warranted here i believe.

nicebyte avatar Aug 11 '24 04:08 nicebyte

I'll have to check when I'm not on a phone, but I think this was intended to deal with platforms that didn't offer recursive mutexes--which Mac OS X didn't at the time--but I think anything we care about does now and we should probably just remove this.

(Although strictly speaking, TSan is incorrect here: if we already hold the mutex, that value will match the current thread ID, otherwise we will wait for the lock. Short of memory corruption, it can't actually get a non-deterministic or racey result, afaict.)

icculus avatar Aug 11 '24 23:08 icculus