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.

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

Thread Sanitizer flags __PHYSFS_platformGrabMutex as having a data race #87

Open
nicebyte opened this issue Aug 11, 2024 · 1 comment
Open

Comments

@nicebyte
Copy link

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.

@icculus
Copy link
Owner

icculus commented Aug 11, 2024

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants