Thread API cleanup

This patch cleans up the thread API and comes with the following
noteworthy changes:

- Introduced Cpu_session::Weight type that replaces a formerly used
  plain integer value to prevent the accidental mix-up of
  arguments.
- The enum definition of Cpu_session::DEFAULT_WEIGHT moved to
  Cpu_session::Weight::DEFAULT_WEIGHT
- New Thread constructor that takes a 'Env &' as first argument.
  The original constructors are now marked as deprecated. For the
  common use case where the default 'Weight' and 'Affinity' are
  used, a shortcut is provided. In the long term, those two
  constructors should be the only ones to remain.
- The former 'Thread<>' class template has been renamed to
  'Thread_deprecated'.
- The former 'Thread_base' class is now called 'Thread'.
- The new 'name()' accessor returns the thread's name as 'Name'
  object as centrally defined via 'Cpu_session::Name'. It is meant to
  replace the old-fashioned 'name' method that takes a buffer and size
  as arguments.
- Adaptation of the thread test to the new API

Issue #1954

repos/base/src/lib/base/child_process.cc

@@ -167,7 +167,7 @@ Child::Process::Initial_thread::Initial_thread(Cpu_session          &cpu,
                                                char           const *name)
 :
 	cpu(cpu),
-	cap(cpu.create_thread(pd, Cpu_session::DEFAULT_WEIGHT, name))
+	cap(cpu.create_thread(pd, name, Affinity::Location(), Cpu_session::Weight()))
 { }
 
 

repos/base/src/lib/base/entrypoint.cc

@@ -12,11 +12,14 @@
  * under the terms of the GNU General Public License version 2.
  */
 
+/* Genode includes */
 #include <base/entrypoint.h>
 #include <base/component.h>
 #include <cap_session/connection.h>
 #include <util/retry.h>
 
+/* base-internal includes */
+#include <base/internal/globals.h>
 
 using namespace Genode;
 
@@ -27,7 +30,6 @@ namespace Genode {
 
 	extern bool inhibit_tracing;
 	void call_global_static_constructors();
-	void init_signal_thread();
 	void destroy_signal_thread();
 
 	extern void (*call_component_construct)(Genode::Env &);
@@ -76,7 +78,7 @@ void Entrypoint::_process_incoming_signals()
 		/* execute fork magic in noux plugin */
 		_suspended_callback();
 
-		init_signal_thread();
+		init_signal_thread(_env);
 		_rpc_ep.construct(&_env.pd(), Component::stack_size(), Component::name());
 		_signal_proxy_cap = manage(_signal_proxy);
 		_sig_rec.construct();
@@ -155,7 +157,7 @@ Entrypoint::Entrypoint(Env &env)
 	_rpc_ep(&env.pd(), Component::stack_size(), Component::name())
 {
 	/* initialize signalling after initializing but before calling the entrypoint */
-	init_signal_thread();
+	init_signal_thread(_env);
 
 	/*
 	 * Invoke Component::construct function in the context of the entrypoint.
@@ -186,6 +188,6 @@ Entrypoint::Entrypoint(Env &env, size_t stack_size, char const *name)
 	_env(env),
 	_rpc_ep(&env.pd(), stack_size, name)
 {
-	_signal_proxy_thread.construct(*this);
+	_signal_proxy_thread.construct(env, *this);
 }
 

repos/base/src/lib/base/lock.cc

@@ -21,13 +21,13 @@
 using namespace Genode;
 
 
-static inline Genode::Thread_base *invalid_thread_base()
+static inline Genode::Thread *invalid_thread_base()
 {
-	return (Genode::Thread_base*)~0;
+	return (Genode::Thread*)~0;
 }
 
 
-static inline bool thread_base_valid(Genode::Thread_base *thread_base)
+static inline bool thread_base_valid(Genode::Thread *thread_base)
 {
 	return (thread_base != invalid_thread_base());
 }
@@ -62,7 +62,7 @@ void Cancelable_lock::Applicant::wake_up()
 
 void Cancelable_lock::lock()
 {
-	Applicant myself(Thread_base::myself());
+	Applicant myself(Thread::myself());
 
 	spinlock_lock(&_spinlock_state);
 

repos/base/src/lib/base/rpc_entrypoint.cc

@@ -60,7 +60,7 @@ void Rpc_entrypoint::activate()
 
 bool Rpc_entrypoint::is_myself() const
 {
-	return (Thread_base::myself() == this);
+	return (Thread::myself() == this);
 }
 
 
@@ -68,13 +68,13 @@ Rpc_entrypoint::Rpc_entrypoint(Pd_session *pd_session, size_t stack_size,
                                char const *name, bool start_on_construction,
                                Affinity::Location location)
 :
-	Thread_base(Cpu_session::DEFAULT_WEIGHT, name, stack_size, location),
+	Thread(Cpu_session::Weight::DEFAULT_WEIGHT, name, stack_size, location),
 	_cap(Untyped_capability()),
 	_cap_valid(Lock::LOCKED), _delay_start(Lock::LOCKED),
 	_delay_exit(Lock::LOCKED),
 	_pd_session(*pd_session)
 {
-	Thread_base::start();
+	Thread::start();
 	_block_until_cap_valid();
 
 	if (start_on_construction)

repos/base/src/lib/base/signal.cc

@@ -11,6 +11,7 @@
  * under the terms of the GNU General Public License version 2.
  */
 
+/* Genode includes */
 #include <util/retry.h>
 #include <base/env.h>
 #include <base/signal.h>
@@ -20,11 +21,12 @@
 #include <signal_source/client.h>
 #include <util/volatile_object.h>
 
+/* base-internal includes */
+#include <base/internal/globals.h>
+
 using namespace Genode;
 
-enum { STACK_SIZE = 4*1024*sizeof(addr_t) };
-
-class Signal_handler_thread : Thread<STACK_SIZE>, Lock
+class Signal_handler_thread : Thread, Lock
 {
 	private:
 
@@ -44,13 +46,15 @@ class Signal_handler_thread : Thread<STACK_SIZE>, Lock
 			Signal_receiver::dispatch_signals(&(*_signal_source));
 		}
 
+		enum { STACK_SIZE = 4*1024*sizeof(addr_t) };
+
 	public:
 
 		/**
 		 * Constructor
 		 */
-		Signal_handler_thread()
-		: Thread<STACK_SIZE>("signal handler"), Lock(Lock::LOCKED)
+		Signal_handler_thread(Env &env)
+		: Thread(env, "signal handler", STACK_SIZE), Lock(Lock::LOCKED)
 		{
 			start();
 
@@ -92,10 +96,10 @@ namespace Genode {
 	 * We allow this function to be overridden in to enable core to omit the
 	 * creation of the signal thread.
 	 */
-	void init_signal_thread() __attribute__((weak));
-	void init_signal_thread()
+	void init_signal_thread(Env &env) __attribute__((weak));
+	void init_signal_thread(Env &env)
 	{
-		signal_handler_thread().construct();
+		signal_handler_thread().construct(env);
 	}
 
 	void destroy_signal_thread()

repos/base/src/lib/base/stack_allocator.cc

@@ -45,7 +45,7 @@ addr_t Stack_allocator::idx_to_base(size_t idx)
 
 
 Stack *
-Stack_allocator::alloc(Thread_base *thread_base, bool main_thread)
+Stack_allocator::alloc(Thread *thread_base, bool main_thread)
 {
 	if (main_thread)
 		/* the main-thread stack is the first one */

repos/base/src/lib/base/thread.cc

@@ -52,7 +52,7 @@ void Stack::size(size_t const size)
 	addr_t const stack_slot_base = Stack_allocator::addr_to_base(this);
 	size_t const ds_size = align_addr(size - stack_size, PAGE_SIZE_LOG2);
 	if (_base - ds_size < stack_slot_base)
-		throw Thread_base::Stack_too_large();
+		throw Thread::Stack_too_large();
 
 	/* allocate and attach backing store for the stack enhancement */
 	addr_t const ds_addr = _base - ds_size - stack_area_virtual_base();
@@ -63,10 +63,10 @@ void Stack::size(size_t const size)
 		void * const attach_addr = rm->attach_at(ds_cap, ds_addr, ds_size);
 
 		if (ds_addr != (addr_t)attach_addr)
-			throw Thread_base::Out_of_stack_space();
+			throw Thread::Out_of_stack_space();
 	}
 	catch (Ram_session::Alloc_failed) {
-		throw Thread_base::Stack_alloc_failed();
+		throw Thread::Stack_alloc_failed();
 	}
 
 	/* update stack information */
@@ -75,7 +75,7 @@ void Stack::size(size_t const size)
 
 
 Stack *
-Thread_base::_alloc_stack(size_t stack_size, char const *name, bool main_thread)
+Thread::_alloc_stack(size_t stack_size, char const *name, bool main_thread)
 {
 	/*
 	 * Synchronize stack list when creating new threads from multiple threads
@@ -134,7 +134,7 @@ Thread_base::_alloc_stack(size_t stack_size, char const *name, bool main_thread)
 }
 
 
-void Thread_base::_free_stack(Stack *stack)
+void Thread::_free_stack(Stack *stack)
 {
 	addr_t ds_addr = stack->base() - stack_area_virtual_base();
 	Ram_dataspace_capability ds_cap = stack->ds_cap();
@@ -150,63 +150,66 @@ void Thread_base::_free_stack(Stack *stack)
 }
 
 
-void Thread_base::name(char *dst, size_t dst_len)
+void Thread::name(char *dst, size_t dst_len)
 {
 	snprintf(dst, dst_len, "%s", _stack->name().string());
 }
 
 
-void Thread_base::join() { _join_lock.lock(); }
+Thread::Name Thread::name() const { return _stack->name(); }
 
 
-void *Thread_base::alloc_secondary_stack(char const *name, size_t stack_size)
+void Thread::join() { _join_lock.lock(); }
+
+
+void *Thread::alloc_secondary_stack(char const *name, size_t stack_size)
 {
 	Stack *stack = _alloc_stack(stack_size, name, false);
 	return (void *)stack->top();
 }
 
 
-void Thread_base::free_secondary_stack(void* stack_addr)
+void Thread::free_secondary_stack(void* stack_addr)
 {
 	addr_t base = Stack_allocator::addr_to_base(stack_addr);
 	_free_stack(Stack_allocator::base_to_stack(base));
 }
 
 
-Native_thread &Thread_base::native_thread() {
+Native_thread &Thread::native_thread() {
 
 	return _stack->native_thread(); }
 
 
-void *Thread_base::stack_top() const { return (void *)_stack->top(); }
+void *Thread::stack_top() const { return (void *)_stack->top(); }
 
 
-void *Thread_base::stack_base() const { return (void*)_stack->base(); }
+void *Thread::stack_base() const { return (void*)_stack->base(); }
 
 
-void Thread_base::stack_size(size_t const size) { _stack->size(size); }
+void Thread::stack_size(size_t const size) { _stack->size(size); }
 
 
-size_t Thread_base::stack_virtual_size()
+size_t Thread::stack_virtual_size()
 {
 	return Genode::stack_virtual_size();
 }
 
 
-addr_t Thread_base::stack_area_virtual_base()
+addr_t Thread::stack_area_virtual_base()
 {
 	return Genode::stack_area_virtual_base();
 }
 
 
-size_t Thread_base::stack_area_virtual_size()
+size_t Thread::stack_area_virtual_size()
 {
 	return Genode::stack_area_virtual_size();
 }
 
 
-Thread_base::Thread_base(size_t weight, const char *name, size_t stack_size,
-                         Type type, Cpu_session *cpu_session, Affinity::Location affinity)
+Thread::Thread(size_t weight, const char *name, size_t stack_size,
+               Type type, Cpu_session *cpu_session, Affinity::Location affinity)
 :
 	_cpu_session(cpu_session),
 	_affinity(affinity),
@@ -225,14 +228,25 @@ Thread_base::Thread_base(size_t weight, const char *name, size_t stack_size,
 }
 
 
-Thread_base::Thread_base(size_t weight, const char *name, size_t stack_size,
-                         Type type, Affinity::Location affinity)
-: Thread_base(weight, name, stack_size, type, nullptr, affinity) { }
+Thread::Thread(size_t weight, const char *name, size_t stack_size,
+               Type type, Affinity::Location affinity)
+: Thread(weight, name, stack_size, type, nullptr, affinity) { }
 
 
-Thread_base::~Thread_base()
+Thread::Thread(Env &env, Name const &name, size_t stack_size, Location location,
+               Weight weight, Cpu_session &cpu)
+: Thread(weight.value, name.string(), stack_size, NORMAL,
+         &cpu == &env.cpu() ? nullptr : &cpu, location)
+{ }
+
+
+Thread::Thread(Env &env, Name const &name, size_t stack_size)
+: Thread(env, name, stack_size, Location(), Weight(), env.cpu()) { }
+
+
+Thread::~Thread()
 {
-	if (Thread_base::myself() == this) {
+	if (Thread::myself() == this) {
 		PERR("thread '%s' tried to self de-struct - sleeping forever.",
 		     _stack->name().string());
 		sleep_forever();

repos/base/src/lib/base/thread_bootstrap.cc

@@ -15,4 +15,4 @@
 
 using namespace Genode;
 
-void Thread_base::_thread_bootstrap() { }
+void Thread::_thread_bootstrap() { }

repos/base/src/lib/base/thread_myself.cc

@@ -19,7 +19,7 @@
 #include <base/internal/stack_area.h>
 
 
-Genode::Thread_base *Genode::Thread_base::myself()
+Genode::Thread *Genode::Thread::myself()
 {
 	int dummy = 0; /* used for determining the stack pointer */
 

repos/base/src/lib/base/thread_start.cc

@@ -26,20 +26,20 @@ using namespace Genode;
 /**
  * Entry point entered by new threads
  */
-void Thread_base::_thread_start()
+void Thread::_thread_start()
 {
-	Thread_base::myself()->_thread_bootstrap();
-	Thread_base::myself()->entry();
-	Thread_base::myself()->_join_lock.unlock();
+	Thread::myself()->_thread_bootstrap();
+	Thread::myself()->entry();
+	Thread::myself()->_join_lock.unlock();
 	Genode::sleep_forever();
 }
 
 
-/*****************
- ** Thread base **
- *****************/
+/************
+ ** Thread **
+ ************/
 
-void Thread_base::_deinit_platform_thread()
+void Thread::_deinit_platform_thread()
 {
 	if (!_cpu_session)
 		_cpu_session = env()->cpu_session();
@@ -48,19 +48,16 @@ void Thread_base::_deinit_platform_thread()
 }
 
 
-void Thread_base::start()
+void Thread::start()
 {
-	/* if no cpu session is given, use it from the environment */
+	/* if no CPU session is given, use it from the environment */
 	if (!_cpu_session)
 		_cpu_session = env()->cpu_session();
 
 	/* create thread at core */
-	char buf[48];
-	name(buf, sizeof(buf));
-	enum { WEIGHT = Cpu_session::DEFAULT_WEIGHT };
 	addr_t const utcb = (addr_t)&_stack->utcb();
-	_thread_cap = _cpu_session->create_thread(env()->pd_session_cap(),
-	                                          WEIGHT, buf, _affinity, utcb);
+	_thread_cap = _cpu_session->create_thread(env()->pd_session_cap(), name(),
+	                                          _affinity, Weight(), utcb);
 	if (!_thread_cap.valid())
 		throw Cpu_session::Thread_creation_failed();
 
@@ -69,7 +66,7 @@ void Thread_base::start()
 }
 
 
-void Thread_base::cancel_blocking()
+void Thread::cancel_blocking()
 {
 	_cpu_session->cancel_blocking(_thread_cap);
 }

repos/base/src/lib/base/trace.cc

@@ -163,9 +163,9 @@ Trace::Logger::Logger()
 { }
 
 
-/*****************
- ** Thread_base **
- *****************/
+/************
+ ** Thread **
+ ************/
 
 /**
  * return logger instance for the main thread **
@@ -180,12 +180,12 @@ static Trace::Logger *main_trace_logger()
 static Trace::Control *main_trace_control;
 
 
-Trace::Logger *Thread_base::_logger()
+Trace::Logger *Thread::_logger()
 {
 	if (inhibit_tracing)
 		return 0;
 
-	Thread_base * const myself = Thread_base::myself();
+	Thread * const myself = Thread::myself();
 
 	Trace::Logger * const logger = myself ? &myself->_trace_logger
 	                                      : main_trace_logger();

repos/base/src/lib/base/utcb.cc

@@ -19,4 +19,4 @@
 #include <base/internal/native_utcb.h>
 
 
-Genode::Native_utcb *Genode::Thread_base::utcb() { return &_stack->utcb(); }
+Genode::Native_utcb *Genode::Thread::utcb() { return &_stack->utcb(); }

repos/base/src/lib/cxx/misc.cc

@@ -12,7 +12,7 @@
  */
 
 #include <base/env.h>
-#include <base/printf.h>
+#include <base/log.h>
 #include <base/stdint.h>
 #include <base/sleep.h>
 #include <base/thread.h>
@@ -23,14 +23,14 @@ using namespace Genode;
 
 extern "C" void __cxa_pure_virtual()
 {
-	PWRN("cxa pure virtual function called, return addr is %p",
-	     __builtin_return_address(0));
+	Genode::warning("cxa pure virtual function called, return addr is ",
+	                __builtin_return_address(0));
 }
 
 
 extern "C" void __pure_virtual()
 {
-	PWRN("pure virtual function called");
+	Genode::warning("pure virtual function called");
 }
 
 
@@ -103,15 +103,16 @@ extern "C" __attribute__((weak)) void raise()
 
 extern "C" void *abort(void)
 {
-	Genode::Thread_base * myself = Genode::Thread_base::myself();
-	char thread_name[64] = { "unknown" };
+	Genode::Thread const * const myself = Genode::Thread::myself();
+	Thread::Name name = "unknown";
 
 	if (myself)
-		myself->name(thread_name, sizeof(thread_name));
-	PWRN("abort called - thread: '%s'", thread_name);
+		name = myself->name();
+
+	Genode::warning("abort called - thread: ", name.string());
 
 	/* Notify the parent of failure */
-	if (!strcmp("main", thread_name, sizeof(thread_name)))
+	if (name != "main")
 		env()->parent()->exit(1);
 
 	sleep_forever();
@@ -125,7 +126,7 @@ extern "C" void *fputc(void) {
 
 
 extern "C" void *fputs(const char *s, void *) {
-	PWRN("C++ runtime: %s", s);
+	Genode::warning("C++ runtime: ", s);
 	return 0;
 }
 
@@ -166,7 +167,7 @@ void *memset(void *s, int c, size_t n)
 
 
 extern "C" void *stderr(void) {
-	PWRN("stderr - not yet implemented");
+	Genode::warning("stderr - not yet implemented");
 	return 0;
 }
 
@@ -180,7 +181,7 @@ FILE *__stderrp;
 
 extern "C" void *strcat(void)
 {
-	PWRN("strcat - not yet implemented");
+	Genode::warning("strcat - not yet implemented");
 	return 0;
 }
 
@@ -214,7 +215,7 @@ extern "C" int strcmp(const char *s1, const char *s2)
  */
 extern "C" int sprintf(char *str, const char *format, ...)
 {
-	PWRN("sprintf - not implemented");
+	Genode::warning("sprintf - not implemented");
 	return 0;
 }
 
@@ -225,5 +226,5 @@ extern "C" int sprintf(char *str, const char *format, ...)
 
 extern "C" __attribute__((weak)) void __stack_chk_fail_local(void)
 {
-	PERR("Violated stack boundary");
+	Genode::error("Violated stack boundary");
 }

repos/base/src/lib/ldso/main.cc

@@ -567,9 +567,9 @@ void Component::construct(Genode::Env &env)
 	try {
 		if (Genode::config()->xml_node().attribute("ld_verbose").has_value("yes")) {
 			PINF("  %lx .. %lx: stack area",
-			     Genode::Thread_base::stack_area_virtual_base(),
-			     Genode::Thread_base::stack_area_virtual_base() +
-			     Genode::Thread_base::stack_area_virtual_size() - 1);
+			     Genode::Thread::stack_area_virtual_base(),
+			     Genode::Thread::stack_area_virtual_base() +
+			     Genode::Thread::stack_area_virtual_size() - 1);
 			dump_link_map(Elf_object::obj_list()->head());
 		}
 	} catch (...) {  }

repos/base/src/lib/startup/init_main_thread.cc

@@ -44,7 +44,7 @@ void init_rtld() { }
 /**
  * The first thread in a program
  */
-class Main_thread : public Thread<MAIN_THREAD_STACK_SIZE>
+class Main_thread : public Thread_deprecated<MAIN_THREAD_STACK_SIZE>
 {
 	public:
 
@@ -55,7 +55,7 @@ class Main_thread : public Thread<MAIN_THREAD_STACK_SIZE>
 		 */
 		Main_thread(bool reinit)
 		:
-			Thread("main", reinit ? REINITIALIZED_MAIN : MAIN)
+			Thread_deprecated("main", reinit ? REINITIALIZED_MAIN : MAIN)
 		{ }
 
 		/**********************