Add 'Thread_base::join()'

Using the new 'join()' function, the caller can explicitly block for the
completion of the thread's 'entry()' function. The test case for this
feature can be found at 'os/src/test/thread_join'. For hybrid
Linux/Genode programs, the 'Thread_base::join()' does not map directly
to 'pthread_join'. The latter function gets already called by the
destructor of 'Thread_base'. According to the documentation, subsequent
calls of 'pthread_join' for one thread may result in undefined behaviour.
So we use a 'Genode::Lock' on this platform, which is in line with the
other platforms.

Related to #194, #501

base-codezero/src/base/thread/thread_start.cc

@@ -30,6 +30,7 @@ void Thread_base::_thread_start()
 {
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	Genode::sleep_forever();
 }
 

base-fiasco/src/core/thread_start.cc

@@ -26,6 +26,7 @@ void Thread_base::_thread_start()
 {
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	sleep_forever();
 }
 

base-foc/src/base/thread/thread.cc

@@ -189,10 +189,17 @@ Thread_base *Thread_base::myself() {
 }
 
 
+void Thread_base::join()
+{
+	_join_lock.lock();
+}
+
+
 Thread_base::Thread_base(const char *name, size_t stack_size)
 :
 	_list_element(this),
-	_context(_alloc_context(stack_size))
+	_context(_alloc_context(stack_size)),
+	_join_lock(Lock::LOCKED)
 {
 	strncpy(_context->name, name, sizeof(_context->name));
 	_init_platform_thread();

base-foc/src/base/thread/thread_bootstrap.cc

@@ -24,6 +24,7 @@ void Genode::Thread_base::_thread_start()
 
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	sleep_forever();
 }
 

base-hw/src/base/thread_support.cc

@@ -42,6 +42,7 @@ void Thread_base::_thread_start()
 {
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	Genode::sleep_forever();
 }
 

base-linux/src/base/thread/thread_linux.cc

@@ -40,7 +40,7 @@ static Lock &startup_lock()
 static void thread_exit_signal_handler(int) { lx_exit(0); }
 
 
-static void thread_start(void *arg)
+void Thread_base::_thread_start()
 {
 	/*
 	 * Set signal handler such that canceled system calls get not
@@ -53,7 +53,7 @@ static void thread_start(void *arg)
 	 */
 	lx_sigaction(LX_SIGCHLD, (void (*)(int))1);
 
-	Thread_base *thread = (Thread_base *)arg;
+	Thread_base * const thread = Thread_base::myself();
 
 	/* inform core about the new thread and process ID of the new thread */
 	Linux_cpu_session *cpu = dynamic_cast<Linux_cpu_session *>(env()->cpu_session());
@@ -63,7 +63,11 @@ static void thread_start(void *arg)
 	/* wakeup 'start' function */
 	startup_lock().unlock();
 
-	Thread_base::myself()->entry();
+	thread->entry();
+
+	/* unblock caller of 'join()' */
+	thread->_join_lock.unlock();
+
 	sleep_forever();
 }
 
@@ -125,7 +129,7 @@ void Thread_base::start()
 
 	/* align initial stack to 16 byte boundary */
 	void *thread_sp = (void *)((addr_t)(_context->stack) & ~0xf);
-	_tid.tid = lx_create_thread(thread_start, thread_sp, this);
+	_tid.tid = lx_create_thread(Thread_base::_thread_start, thread_sp, this);
 	_tid.pid = lx_getpid();
 
 	/* wait until the 'thread_start' function got entered */

base-linux/src/core/thread_linux.cc

@@ -24,7 +24,7 @@ using namespace Genode;
 static void empty_signal_handler(int) { }
 
 
-static void thread_start(void *)
+void Thread_base::_thread_start()
 {
 	/*
 	 * Set signal handler such that canceled system calls get not
@@ -52,7 +52,7 @@ void Thread_base::start()
 {
 	/* align initial stack to 16 byte boundary */
 	void *thread_sp = (void *)((addr_t)(_context->stack) & ~0xf);
-	_tid.tid = lx_create_thread(thread_start, thread_sp, this);
+	_tid.tid = lx_create_thread(Thread_base::_thread_start, thread_sp, this);
 	_tid.pid = lx_getpid();
 }
 

base-linux/src/platform/linux_syscalls.h

@@ -306,7 +306,7 @@ inline int lx_tgkill(int pid, int tid, int signal)
 }
 
 
-inline int lx_create_thread(void (*entry)(void *), void *stack, void *arg)
+inline int lx_create_thread(void (*entry)(), void *stack, void *arg)
 {
 	int flags = CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND
 	          | CLONE_THREAD | CLONE_SYSVSEM;

base-linux/src/platform/lx_hybrid.cc

@@ -130,16 +130,26 @@ namespace Genode {
 
 	struct Thread_meta_data
 	{
+		/**
+		 * Lock with the initial state set to LOCKED
+		 */
+		struct Barrier : Lock { Barrier() : Lock(Lock::LOCKED) { } };
+
 		/**
 		 * Used to block the constructor until the new thread has initialized
 		 * 'id'
 		 */
-		Lock construct_lock;
+		Barrier construct_lock;
 
 		/**
 		 * Used to block the new thread until 'start' is called
 		 */
-		Lock start_lock;
+		Barrier start_lock;
+
+		/**
+		 * Used to block the 'join()' function until the 'entry()' is done
+		 */
+		Barrier join_lock;
 
 		/**
 		 * Filled out by 'thread_start' function in the context of the new
@@ -155,13 +165,9 @@ namespace Genode {
 		/**
 		 * Constructor
 		 *
-		 * \param thread_base  associated 'Thread_base' object
+		 * \param thread  associated 'Thread_base' object
 		 */
-		Thread_meta_data(Thread_base *thread_base)
+		Thread_meta_data(Thread_base *thread) : thread_base(thread) { }
-		:
-			construct_lock(Lock::LOCKED), start_lock(Lock::LOCKED),
-			thread_base(thread_base)
-		{ }
 	};
 }
 
@@ -224,6 +230,8 @@ static void *thread_start(void *arg)
 	meta_data->start_lock.lock();
 
 	Thread_base::myself()->entry();
+
+	meta_data->join_lock.unlock();
 	return 0;
 }
 
@@ -286,8 +294,15 @@ void Thread_base::start()
 }
 
 
+void Thread_base::join()
+{
+	_tid.meta_data->join_lock.lock();
+}
+
+
 Thread_base::Thread_base(const char *name, size_t stack_size)
-: _list_element(this)
+:
+	_list_element(this)
 {
 	_tid.meta_data = new (env()->heap()) Thread_meta_data(this);
 
@@ -325,13 +340,9 @@ void Thread_base::cancel_blocking()
 
 Thread_base::~Thread_base()
 {
-	{
+	bool const needs_join = (pthread_cancel(_tid.meta_data->pt) == 0);
-		int const ret = pthread_cancel(_tid.meta_data->pt);
-		if (ret)
-			PWRN("pthread_cancel unexpectedly returned with %d", ret);
-	}
 
-	{
+	if (needs_join) {
 		int const ret = pthread_join(_tid.meta_data->pt, 0);
 		if (ret)
 			PWRN("pthread_join unexpectedly returned with %d (errno=%d)",

base-mb/src/base/thread/thread_start.cc

@@ -27,6 +27,7 @@ void Thread_base::_thread_start()
 {
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	Genode::sleep_forever();
 }
 

base-mb/src/core/thread_roottask.cc

@@ -58,6 +58,7 @@ void Thread_base::_thread_start()
 	PDBG("Thread returned, tid=%i, pid=%i",
 	     myself()->tid(), Roottask::PROTECTION_ID);
 
+	Thread_base::myself()->_join_lock.unlock();
 	Genode::sleep_forever();
 }
 

base-nova/src/base/thread/thread_nova.cc

@@ -37,6 +37,7 @@ using namespace Genode;
 void Thread_base::_thread_start()
 {
 	Genode::Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	Genode::sleep_forever();
 }
 

base-okl4/src/core/thread_start.cc

@@ -26,6 +26,7 @@ void Thread_base::_thread_start()
 {
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	sleep_forever();
 }
 

base-pistachio/src/core/thread_start.cc

@@ -26,6 +26,7 @@ void Thread_base::_thread_start()
 {
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	sleep_forever();
 }
 

base/include/base/thread.h

@@ -254,6 +254,11 @@ namespace Genode {
 			 */
 			Native_thread _tid;
 
+			/**
+			 * Lock used for synchronizing the finalization of the thread
+			 */
+			Genode::Lock _join_lock;
+
 		public:
 
 			/**
@@ -335,6 +340,14 @@ namespace Genode {
 			 * 0 when called by the main thread.
 			 */
 			Native_utcb *utcb();
+
+			/**
+			 * Block until the thread leaves the 'entry' function
+			 *
+			 * Join must not be called more than once. Subsequent calls have
+			 * undefined behaviour.
+			 */
+			void join();
 	};
 
 

base/src/base/thread/thread.cc

@@ -195,10 +195,17 @@ Thread_base *Thread_base::myself()
 }
 
 
+void Thread_base::join()
+{
+	_join_lock.lock();
+}
+
+
 Thread_base::Thread_base(const char *name, size_t stack_size)
 :
 	_list_element(this),
-	_context(_alloc_context(stack_size))
+	_context(_alloc_context(stack_size)),
+	_join_lock(Lock::LOCKED)
 {
 	strncpy(_context->name, name, sizeof(_context->name));
 	_init_platform_thread();

base/src/base/thread/thread_start.cc

@@ -27,6 +27,7 @@ void Thread_base::_thread_start()
 {
 	Thread_base::myself()->_thread_bootstrap();
 	Thread_base::myself()->entry();
+	Thread_base::myself()->_join_lock.unlock();
 	Genode::sleep_forever();
 }
 

os/run/thread_join.run

@@ -0,0 +1,39 @@
+build "core init drivers/timer test/thread_join"
+
+create_boot_directory
+
+install_config {
+	<config>
+		<parent-provides>
+			<service name="ROM"/>
+			<service name="RAM"/>
+			<service name="CPU"/>
+			<service name="RM"/>
+			<service name="CAP"/>
+			<service name="PD"/>
+			<service name="IRQ"/>
+			<service name="IO_PORT"/>
+			<service name="IO_MEM"/>
+			<service name="SIGNAL"/>
+			<service name="LOG"/>
+		</parent-provides>
+		<default-route>
+			<any-service> <parent/> <any-child/> </any-service>
+		</default-route>
+		<start name="timer">
+			<resource name="RAM" quantum="1M"/>
+			<provides><service name="Timer"/></provides>
+		</start>
+		<start name="test-thread_join">
+			<resource name="RAM" quantum="10M"/>
+		</start>
+	</config>
+}
+
+build_boot_image "core init timer test-thread_join"
+
+append qemu_args "-nographic -m 64"
+
+run_genode_until {child exited with exit value 0.*} 10
+
+puts "Test succeeded"

os/src/test/thread_join/main.cc

@@ -0,0 +1,74 @@
+/*
+ * \brief  Test for the 'Thread_base::join()' function
+ * \author Norman Feske
+ * \date   2012-11-16
+ */
+
+/*
+ * Copyright (C) 2012 Genode Labs GmbH
+ *
+ * This file is part of the Genode OS framework, which is distributed
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#include <base/printf.h>
+#include <base/thread.h>
+#include <timer_session/connection.h>
+
+using namespace Genode;
+
+
+struct Worker : Genode::Thread<4096>
+{
+	Timer::Session   &timer;
+	unsigned    const result_value;
+	unsigned volatile result;
+
+	void entry()
+	{
+		PLOG("worker thread is up");
+		timer.msleep(250);
+
+		PLOG("worker is leaving the entry function with result=%u...",
+		     result_value);
+		result = result_value;
+	}
+
+	Worker(Timer::Session &timer, int result_value)
+	:
+		timer(timer), result_value(result_value), result(~0)
+	{
+		start();
+	}
+};
+
+
+/**
+ * Main program
+ */
+int main(int, char **)
+{
+	printf("--- thread join test ---\n");
+
+	Timer::Connection timer;
+
+	for (unsigned i = 0; i < 10; i++) {
+
+		/*
+		 * A worker thread is created in each iteration. Just before
+		 * leaving the entry function, the worker assigns the result
+		 * to 'Worker::result' variable. By validating this value,
+		 * we determine whether the worker has finished or not.
+		 */
+		Worker worker(timer, i);
+		worker.join();
+
+		if (worker.result != i) {
+			PERR("work remains unfinished after 'join()' returned");
+			return -1;
+		}
+	}
+
+	printf("--- signalling test finished ---\n");
+	return 0;
+}

os/src/test/thread_join/target.mk

@@ -0,0 +1,3 @@
+TARGET = test-thread_join
+SRC_CC = main.cc
+LIBS   = cxx env thread

tool/autopilot.list

@@ -15,3 +15,4 @@ lx_hybrid_ctors
 lx_hybrid_exception
 lx_hybrid_pthread_ipc
 moon
+thread_join