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

Fix fake recursive pthread locks #5479

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 50 additions & 0 deletions src/thread/pthread/SDL_syscond.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
#include "SDL_thread.h"
#include "SDL_sysmutex_c.h"

#if !SDL_THREAD_PTHREAD_RECURSIVE_MUTEX && \
!SDL_THREAD_PTHREAD_RECURSIVE_MUTEX_NP
#define FAKE_RECURSIVE_MUTEX 1
#endif

struct SDL_cond
{
pthread_cond_t cond;
Expand Down Expand Up @@ -103,6 +108,10 @@ SDL_CondWaitTimeout(SDL_cond * cond, SDL_mutex * mutex, Uint32 ms)
struct timeval delta;
#endif
struct timespec abstime;
#ifdef FAKE_RECURSIVE_MUTEX
pthread_t this_thread = pthread_self();
int prev;
#endif

if (!cond) {
return SDL_InvalidParamError("cond");
Expand All @@ -125,7 +134,22 @@ SDL_CondWaitTimeout(SDL_cond * cond, SDL_mutex * mutex, Uint32 ms)
}

tryagain:
#ifdef FAKE_RECURSIVE_MUTEX
// Reset recursive mutex state while waiting on condition variable,
// allowing other threads to acquire the mutex.
prev = mutex->recursive;
mutex->recursive = 0;
mutex->owner = 0;
pthread_cond_signal(&mutex->rec_cond);

retval = pthread_cond_timedwait(&cond->cond, &mutex->id, &abstime);

// Restore recursive mutex state.
mutex->recursive = prev;
mutex->owner = pthread_self();
#else
retval = pthread_cond_timedwait(&cond->cond, &mutex->id, &abstime);
#endif
switch (retval) {
case EINTR:
goto tryagain;
Expand All @@ -147,12 +171,38 @@ SDL_CondWaitTimeout(SDL_cond * cond, SDL_mutex * mutex, Uint32 ms)
int
SDL_CondWait(SDL_cond * cond, SDL_mutex * mutex)
{
#ifdef FAKE_RECURSIVE_MUTEX
int retval;

if (!cond) {
return SDL_InvalidParamError("cond");
}

// Reset recursive mutex state while waiting on condition variable,
// allowing other threads to acquire the mutex.
int prev = mutex->recursive;
mutex->recursive = 0;
mutex->owner = 0;
pthread_cond_signal(&mutex->rec_cond);

retval = pthread_cond_wait(&cond->cond, &mutex->id);

// Restore recursive mutex state.
mutex->recursive = prev;
mutex->owner = pthread_self();

if (retval != 0) {
return SDL_SetError("pthread_cond_wait() failed");
}
return 0;
#else
if (!cond) {
return SDL_InvalidParamError("cond");
} else if (pthread_cond_wait(&cond->cond, &mutex->id) != 0) {
return SDL_SetError("pthread_cond_wait() failed");
}
return 0;
#endif
}

/* vi: set ts=4 sw=4 expandtab: */
87 changes: 50 additions & 37 deletions src/thread/pthread/SDL_sysmutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ struct SDL_mutex
#if FAKE_RECURSIVE_MUTEX
int recursive;
pthread_t owner;
pthread_mutex_t rec_id;
pthread_cond_t rec_cond;
#endif
};

Expand All @@ -61,6 +63,18 @@ SDL_CreateMutex(void)
SDL_free(mutex);
mutex = NULL;
}
#ifdef FAKE_RECURSIVE_MUTEX
if (mutex && pthread_mutex_init(&mutex->rec_id, &attr) != 0) {
SDL_SetError("pthread_mutex_init() failed");
SDL_free(mutex);
mutex = NULL;
}
if (mutex && pthread_cond_init(&mutex->rec_cond, NULL) != 0) {
SDL_SetError("pthread_cond_init() failed");
SDL_free(mutex);
mutex = NULL;
}
#endif
} else {
SDL_OutOfMemory();
}
Expand All @@ -72,6 +86,10 @@ SDL_DestroyMutex(SDL_mutex * mutex)
{
if (mutex) {
pthread_mutex_destroy(&mutex->id);
#ifdef FAKE_RECURSIVE_MUTEX
pthread_mutex_destroy(&mutex->rec_id);
pthread_cond_destroy(&mutex->rec_cond);
#endif
SDL_free(mutex);
}
}
Expand All @@ -90,19 +108,18 @@ SDL_LockMutex(SDL_mutex * mutex)

#if FAKE_RECURSIVE_MUTEX
this_thread = pthread_self();
if (mutex->owner == this_thread) {
if (pthread_mutex_lock(&mutex->rec_id) == 0) {
while (mutex->owner && mutex->owner != this_thread) {
pthread_cond_wait(&mutex->rec_cond, &mutex->rec_id);
}
if (mutex->recursive == 0) {
pthread_mutex_lock(&mutex->id);
}
mutex->owner = this_thread;
++mutex->recursive;
pthread_mutex_unlock(&mutex->rec_id);
} else {
/* The order of operations is important.
We set the locking thread id after we obtain the lock
so unlocks from other threads will fail.
*/
if (pthread_mutex_lock(&mutex->id) == 0) {
mutex->owner = this_thread;
mutex->recursive = 0;
} else {
return SDL_SetError("pthread_mutex_lock() failed");
}
return SDL_SetError("pthread_mutex_lock() failed");
}
#else
if (pthread_mutex_lock(&mutex->id) != 0) {
Expand All @@ -128,22 +145,22 @@ SDL_TryLockMutex(SDL_mutex * mutex)
retval = 0;
#if FAKE_RECURSIVE_MUTEX
this_thread = pthread_self();
if (mutex->owner == this_thread) {
result = pthread_mutex_trylock(&mutex->rec_id);
if (result == 0) {
if (mutex->owner && mutex->owner != this_thread) {
pthread_mutex_unlock(&mutex->rec_id);
return SDL_MUTEX_TIMEDOUT;
}
if (mutex->recursive == 0) {
pthread_mutex_lock(&mutex->id);
}
mutex->owner = this_thread;
++mutex->recursive;
pthread_mutex_unlock(&mutex->rec_id);
} else if (result == EBUSY) {
retval = SDL_MUTEX_TIMEDOUT;
} else {
/* The order of operations is important.
We set the locking thread id after we obtain the lock
so unlocks from other threads will fail.
*/
result = pthread_mutex_trylock(&mutex->id);
if (result == 0) {
mutex->owner = this_thread;
mutex->recursive = 0;
} else if (result == EBUSY) {
retval = SDL_MUTEX_TIMEDOUT;
} else {
retval = SDL_SetError("pthread_mutex_trylock() failed");
}
retval = SDL_SetError("pthread_mutex_trylock() failed");
}
#else
result = pthread_mutex_trylock(&mutex->id);
Expand All @@ -166,23 +183,19 @@ SDL_UnlockMutex(SDL_mutex * mutex)
}

#if FAKE_RECURSIVE_MUTEX
/* We can only unlock the mutex if we own it */
if (pthread_self() == mutex->owner) {
if (mutex->recursive) {
--mutex->recursive;
} else {
/* The order of operations is important.
First reset the owner so another thread doesn't lock
the mutex and set the ownership before we reset it,
then release the lock semaphore.
*/
if (pthread_mutex_lock(&mutex->rec_id) == 0) {
SDL_assert(mutex->owner == pthread_self());
--mutex->recursive;
SDL_assert(mutex->recursive >= 0);
if (mutex->recursive == 0) {
mutex->owner = 0;
pthread_mutex_unlock(&mutex->id);
pthread_cond_signal(&mutex->rec_cond);
}
pthread_mutex_unlock(&mutex->rec_id);
} else {
return SDL_SetError("mutex not owned by this thread");
return SDL_SetError("pthread_mutex_lock() failed");
}

#else
if (pthread_mutex_unlock(&mutex->id) != 0) {
return SDL_SetError("pthread_mutex_unlock() failed");
Expand Down
11 changes: 11 additions & 0 deletions src/thread/pthread/SDL_sysmutex_c.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,20 @@
#ifndef SDL_mutex_c_h_
#define SDL_mutex_c_h_

#if !SDL_THREAD_PTHREAD_RECURSIVE_MUTEX && \
!SDL_THREAD_PTHREAD_RECURSIVE_MUTEX_NP
#define FAKE_RECURSIVE_MUTEX 1
#endif

struct SDL_mutex
{
pthread_mutex_t id;
#if FAKE_RECURSIVE_MUTEX
int recursive;
pthread_t owner;
pthread_mutex_t rec_id;
pthread_cond_t rec_cond;
#endif
};

#endif /* SDL_mutex_c_h_ */
Expand Down