[All - Fix] Improved threading safety, notably on macOS (#130)

parsec-cloud · Oct 9, 2024 · afdb89d · afdb89d
1 parent 45a7fdd
commit afdb89d
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 7 deletions.
diff --git a/src/thread.c b/src/thread.c
@@ -55,17 +55,21 @@ void MTY_RWLockDestroy(MTY_RWLock **rwlock)
 		return;
 
 	MTY_RWLock *ctx = *rwlock;
+	*rwlock = NULL;
+	MTY_MEMORY_BARRIER();
 
 	mty_rwlock_destroy(&ctx->rwlock);
 	memset(&RWLOCK_STATE[ctx->index], 0, sizeof(struct thread_rwlock));
 	MTY_Atomic32Set(&RWLOCK_INIT[ctx->index], 0);
 
 	MTY_Free(ctx);
-	*rwlock = NULL;
 }
 
 bool MTY_RWTryLockReader(MTY_RWLock *ctx)
 {
+	if (!ctx)
+		return false;
+
 	struct thread_rwlock *rw = &RWLOCK_STATE[ctx->index];
 
 	bool r = true;
@@ -85,6 +89,9 @@ bool MTY_RWTryLockReader(MTY_RWLock *ctx)
 
 void MTY_RWLockReader(MTY_RWLock *ctx)
 {
+	if (!ctx)
+		return;
+
 	struct thread_rwlock *rw = &RWLOCK_STATE[ctx->index];
 
 	if (rw->taken == 0) {
@@ -99,6 +106,9 @@ void MTY_RWLockReader(MTY_RWLock *ctx)
 
 void MTY_RWLockWriter(MTY_RWLock *ctx)
 {
+	if (!ctx)
+		return;
+
 	bool relock = false;
 	struct thread_rwlock *rw = &RWLOCK_STATE[ctx->index];
 
@@ -120,6 +130,9 @@ void MTY_RWLockWriter(MTY_RWLock *ctx)
 
 void MTY_RWLockUnlock(MTY_RWLock *ctx)
 {
+	if (!ctx)
+		return;
+
 	struct thread_rwlock *rw = &RWLOCK_STATE[ctx->index];
 
 	if (--rw->taken == 0) {
@@ -159,16 +172,20 @@ void MTY_WaitableDestroy(MTY_Waitable **waitable)
 		return;
 
 	MTY_Waitable *ctx = *waitable;
+	*waitable = NULL;
+	MTY_MEMORY_BARRIER();
 
 	MTY_CondDestroy(&ctx->cond);
 	MTY_MutexDestroy(&ctx->mutex);
 
 	MTY_Free(ctx);
-	*waitable = NULL;
 }
 
 bool MTY_WaitableWait(MTY_Waitable *ctx, int32_t timeout)
 {
+	if (!ctx)
+		return false;
+
 	MTY_MutexLock(ctx->mutex);
 
 	if (!ctx->signal)
@@ -184,6 +201,9 @@ bool MTY_WaitableWait(MTY_Waitable *ctx, int32_t timeout)
 
 void MTY_WaitableSignal(MTY_Waitable *ctx)
 {
+	if (!ctx)
+		return;
+
 	MTY_MutexLock(ctx->mutex);
 
 	if (!ctx->signal) {
@@ -232,6 +252,8 @@ void MTY_ThreadPoolDestroy(MTY_ThreadPool **pool, MTY_AnonFunc detach)
 		return;
 
 	MTY_ThreadPool *ctx = *pool;
+	*pool = NULL;
+	MTY_MEMORY_BARRIER();
 
 	for (uint32_t x = 0; x < ctx->num; x++) {
 		MTY_ThreadPoolDetach(ctx, x, detach);
@@ -244,7 +266,6 @@ void MTY_ThreadPoolDestroy(MTY_ThreadPool **pool, MTY_AnonFunc detach)
 
 	MTY_Free(ctx->ti);
 	MTY_Free(ctx);
-	*pool = NULL;
 }
 
 static void *thread_pool_func(void *opaque)
@@ -270,6 +291,9 @@ static void *thread_pool_func(void *opaque)
 
 uint32_t MTY_ThreadPoolDispatch(MTY_ThreadPool *ctx, MTY_AnonFunc func, void *opaque)
 {
+	if (!ctx)
+		return 0;
+
 	uint32_t index = 0;
 
 	for (uint32_t x = 1; x < ctx->num && index == 0; x++) {
@@ -299,6 +323,9 @@ uint32_t MTY_ThreadPoolDispatch(MTY_ThreadPool *ctx, MTY_AnonFunc func, void *op
 
 void MTY_ThreadPoolDetach(MTY_ThreadPool *ctx, uint32_t index, MTY_AnonFunc detach)
 {
+	if (!ctx)
+		return;
+
 	struct thread_info *ti = &ctx->ti[index];
 
 	MTY_MutexLock(ti->m);
@@ -318,6 +345,9 @@ void MTY_ThreadPoolDetach(MTY_ThreadPool *ctx, uint32_t index, MTY_AnonFunc deta
 
 MTY_Async MTY_ThreadPoolPoll(MTY_ThreadPool *ctx, uint32_t index, void **opaque)
 {
+	if (!ctx)
+		return MTY_ASYNC_ERROR;
+
 	struct thread_info *ti = &ctx->ti[index];
 
 	MTY_MutexLock(ti->m);

diff --git a/src/unix/apple/ws.m b/src/unix/apple/ws.m
@@ -153,10 +153,14 @@ void MTY_WebSocketDestroy(MTY_WebSocket **webSocket)
 		[ctx->task cancelWithCloseCode:NSURLSessionWebSocketCloseCodeNormalClosure reason:nil];
 
 	if (ctx->session) {
+		id delegate = [ctx->session delegate];
 		[ctx->session finishTasksAndInvalidate];
 
 		if (ctx->conn)
 			MTY_WaitableWait(ctx->conn, INT32_MAX);
+
+		if (delegate)
+			OBJC_CTX_CLEAR(delegate);
 	}
 
 	ctx->session = nil;

diff --git a/src/unix/thread.c b/src/unix/thread.c
@@ -123,24 +123,31 @@ void MTY_MutexDestroy(MTY_Mutex **mutex)
 		return;
 
 	MTY_Mutex *ctx = *mutex;
+	*mutex = NULL;
+	MTY_MEMORY_BARRIER();
 
 	int32_t e = pthread_mutex_destroy(&ctx->mutex);
 	if (e != 0)
 		MTY_LogFatal("'pthread_mutex_destroy' failed with error %d", e);
 
 	MTY_Free(ctx);
-	*mutex = NULL;
 }
 
 void MTY_MutexLock(MTY_Mutex *ctx)
 {
+	if (!ctx)
+		return;
+
 	int32_t e = pthread_mutex_lock(&ctx->mutex);
 	if (e != 0)
 		MTY_LogFatal("'pthread_mutex_lock' failed with error %d", e);
 }
 
 bool MTY_MutexTryLock(MTY_Mutex *ctx)
 {
+	if (!ctx)
+		return false;
+
 	int32_t e = pthread_mutex_trylock(&ctx->mutex);
 	if (e != 0 && e != EBUSY)
 		MTY_LogFatal("'pthread_mutex_trylock' failed with error %d", e);
@@ -150,6 +157,9 @@ bool MTY_MutexTryLock(MTY_Mutex *ctx)
 
 void MTY_MutexUnlock(MTY_Mutex *ctx)
 {
+	if (!ctx)
+		return;
+
 	int32_t e = pthread_mutex_unlock(&ctx->mutex);
 	if (e != 0)
 		MTY_LogFatal("'pthread_mutex_unlock' failed with error %d", e);
@@ -179,17 +189,21 @@ void MTY_CondDestroy(MTY_Cond **cond)
 		return;
 
 	MTY_Cond *ctx = *cond;
+	*cond = NULL;
+	MTY_MEMORY_BARRIER();
 
 	int32_t e = pthread_cond_destroy(&ctx->cond);
 	if (e != 0)
 		MTY_LogFatal("'pthread_cond_destroy' failed with error %d", e);
 
 	MTY_Free(ctx);
-	*cond = NULL;
 }
 
 bool MTY_CondWait(MTY_Cond *ctx, MTY_Mutex *mutex, int32_t timeout)
 {
+	if (!ctx || !mutex)
+		return false;
+
 	// Use pthread_cond_timedwait
 	if (timeout >= 0) {
 		struct timespec ts = {0};
@@ -222,13 +236,19 @@ bool MTY_CondWait(MTY_Cond *ctx, MTY_Mutex *mutex, int32_t timeout)
 
 void MTY_CondSignal(MTY_Cond *ctx)
 {
+	if (!ctx)
+		return;
+
 	int32_t e = pthread_cond_signal(&ctx->cond);
 	if (e != 0)
 		MTY_LogFatal("'pthread_cond_signal' failed with error %d", e);
 }
 
 void MTY_CondSignalAll(MTY_Cond *ctx)
 {
+	if (!ctx)
+		return;
+
 	int32_t e = pthread_cond_broadcast(&ctx->cond);
 	if (e != 0)
 		MTY_LogFatal("'pthread_cond_broadcast' failed with error %d", e);

diff --git a/src/windows/threadw.c b/src/windows/threadw.c
@@ -116,25 +116,35 @@ void MTY_MutexDestroy(MTY_Mutex **mutex)
 		return;
 
 	MTY_Mutex *ctx = *mutex;
+	*mutex = NULL;
+	MTY_MEMORY_BARRIER();
 
 	DeleteCriticalSection(&ctx->mutex);
 
 	MTY_Free(ctx);
-	*mutex = NULL;
 }
 
 void MTY_MutexLock(MTY_Mutex *ctx)
 {
+	if (!ctx)
+		return;
+
 	EnterCriticalSection(&ctx->mutex);
 }
 
 bool MTY_MutexTryLock(MTY_Mutex *ctx)
 {
+	if (!ctx)
+		return false;
+
 	return TryEnterCriticalSection(&ctx->mutex);
 }
 
 void MTY_MutexUnlock(MTY_Mutex *ctx)
 {
+	if (!ctx)
+		return;
+
 	LeaveCriticalSection(&ctx->mutex);
 }
 
@@ -160,13 +170,17 @@ void MTY_CondDestroy(MTY_Cond **cond)
 		return;
 
 	MTY_Cond *ctx = *cond;
+	*cond = NULL;
+	MTY_MEMORY_BARRIER();
 
 	MTY_Free(ctx);
-	*cond = NULL;
 }
 
 bool MTY_CondWait(MTY_Cond *ctx, MTY_Mutex *mutex, int32_t timeout)
 {
+	if (!ctx || !mutex)
+		return false;
+
 	bool r = true;
 	timeout = timeout < 0 ? INFINITE : timeout;
 
@@ -188,11 +202,17 @@ bool MTY_CondWait(MTY_Cond *ctx, MTY_Mutex *mutex, int32_t timeout)
 
 void MTY_CondSignal(MTY_Cond *ctx)
 {
+	if (!ctx)
+		return;
+
 	WakeConditionVariable(&ctx->cond);
 }
 
 void MTY_CondSignalAll(MTY_Cond *ctx)
 {
+	if (!ctx)
+		return;
+
 	WakeAllConditionVariable(&ctx->cond);
 }