From 35a679d86195cd7920152fd1642756358a99acdf Mon Sep 17 00:00:00 2001
From: Christian Prochaska <christian.prochaska@genode-labs.com>
Date: Tue, 26 Nov 2024 01:08:19 +0100
Subject: [PATCH] libc: pthread cond/rwlock improvements

- add a check to detect if a different thread has
  initialized the internal object in the meantime
- remove ENOMEM error since the 'Libc::Allocator'
  is not supposed to throw exceptions
- remove init mutex from 'pthread_condattr_init()'
  since there is no implicit initialization which
  could happen in parallel like with mutex/cond/rwlock

Issue #5386
---
 repos/libports/src/lib/libc/pthread.cc | 53 ++++++++++++++++----------
 repos/libports/src/lib/libc/rwlock.cc  | 30 +++++++++------
 2 files changed, 51 insertions(+), 32 deletions(-)

diff --git a/repos/libports/src/lib/libc/pthread.cc b/repos/libports/src/lib/libc/pthread.cc
index 49fb0d90d5..644db5cb4f 100644
--- a/repos/libports/src/lib/libc/pthread.cc
+++ b/repos/libports/src/lib/libc/pthread.cc
@@ -1086,6 +1086,15 @@ extern "C" {
 		sem_t           signal_sem;
 		sem_t           handshake_sem;
 
+		struct Invalid_timedwait_clock { };
+
+		void _cleanup()
+		{
+			sem_destroy(&handshake_sem);
+			sem_destroy(&signal_sem);
+			pthread_mutex_destroy(&counter_mutex);
+		}
+
 		pthread_cond(clockid_t clock_id) : num_waiters(0), num_signallers(0)
 		{
 			pthread_mutex_init(&counter_mutex, nullptr);
@@ -1093,17 +1102,12 @@ extern "C" {
 			sem_init(&handshake_sem, 0, 0);
 
 			if (sem_set_clock(&signal_sem, clock_id)) {
-				struct Invalid_timedwait_clock { };
+				_cleanup();
 				throw Invalid_timedwait_clock();
 			}
 		}
 
-		~pthread_cond()
-		{
-			sem_destroy(&handshake_sem);
-			sem_destroy(&signal_sem);
-			pthread_mutex_destroy(&counter_mutex);
-		}
+		~pthread_cond() { _cleanup(); }
 	};
 
 
@@ -1115,17 +1119,11 @@ extern "C" {
 
 	int pthread_condattr_init(pthread_condattr_t *attr)
 	{
-		static Mutex condattr_init_mutex { };
-
 		if (!attr)
 			return EINVAL;
 
-		try {
-			Mutex::Guard guard(condattr_init_mutex);
-			Libc::Allocator alloc { };
-			*attr = new (alloc) pthread_cond_attr;
-			return 0;
-		} catch (...) { return ENOMEM; }
+		Libc::Allocator alloc { };
+		*attr = new (alloc) pthread_cond_attr;
 
 		return 0;
 	}
@@ -1161,22 +1159,35 @@ extern "C" {
 	{
 		static Mutex cond_init_mutex { };
 
-		if (!cond)
-			return EINVAL;
+		Mutex::Guard guard(cond_init_mutex);
+
+		/*
+		 * '*cond' could have been initialized by a different
+		 * thread which got the init mutex earlier.
+		 */
+		if (*cond != PTHREAD_COND_INITIALIZER)
+			return 0;
+
+		Libc::Allocator alloc { };
 
 		try {
-			Mutex::Guard guard(cond_init_mutex);
-			Libc::Allocator alloc { };
 			*cond = attr && *attr ? new (alloc) pthread_cond((*attr)->clock_id)
 			                      : new (alloc) pthread_cond(CLOCK_REALTIME);
-			return 0;
-		} catch (...) { return ENOMEM; }
+		} catch (pthread_cond::Invalid_timedwait_clock) { return EINVAL; }
+
+		return 0;
 	}
 
 
 	int pthread_cond_init(pthread_cond_t *__restrict cond,
 	                      const pthread_condattr_t *__restrict attr)
 	{
+		if (!cond)
+			return EINVAL;
+
+		/* mark as uninitialized for 'cond_init()' */
+		*cond = PTHREAD_COND_INITIALIZER;
+
 		return cond_init(cond, attr);
 	}
 
diff --git a/repos/libports/src/lib/libc/rwlock.cc b/repos/libports/src/lib/libc/rwlock.cc
index e29626b682..e4b1c87b7d 100644
--- a/repos/libports/src/lib/libc/rwlock.cc
+++ b/repos/libports/src/lib/libc/rwlock.cc
@@ -104,20 +104,30 @@ extern "C" {
 	{
 		static Mutex rwlock_init_mutex { };
 
-		if (!rwlock)
-			return EINVAL;
+		Mutex::Guard g(rwlock_init_mutex);
 
-		try {
-			Mutex::Guard g(rwlock_init_mutex);
-			Libc::Allocator alloc { };
-			*rwlock = new (alloc) struct pthread_rwlock();
+		/*
+		 * '*rwlock' could have been initialized by a different
+		 * thread which got the init mutex earlier.
+		 */
+		if (*rwlock != PTHREAD_RWLOCK_INITIALIZER)
 			return 0;
-		} catch (...) { return ENOMEM; }
+
+		Libc::Allocator alloc { };
+		*rwlock = new (alloc) struct pthread_rwlock();
+
+		return 0;
 	}
 
 
 	int pthread_rwlock_init(pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *attr)
 	{
+		if (!rwlock)
+			return EINVAL;
+
+		/* mark as uninitialized for 'rwlock_init()' */
+		*rwlock = PTHREAD_RWLOCK_INITIALIZER;
+
 		return rwlock_init(rwlock, attr);
 	}
 
@@ -142,8 +152,7 @@ extern "C" {
 			return EINVAL;
 
 		if (*rwlock == PTHREAD_RWLOCK_INITIALIZER)
-			if (rwlock_init(rwlock, NULL))
-				return ENOMEM;
+			rwlock_init(rwlock, NULL);
 
 		(*rwlock)->rdlock();
 		return 0;
@@ -159,8 +168,7 @@ extern "C" {
 			return EINVAL;
 
 		if (*rwlock == PTHREAD_RWLOCK_INITIALIZER)
-			if (rwlock_init(rwlock, NULL))
-				return ENOMEM;
+			rwlock_init(rwlock, NULL);
 
 		(*rwlock)->wrlock();
 		return 0;