From e2681805f3cf98852b4e9638c599596cdbf9413e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kochma=C5=84ski?= Date: Fri, 6 Dec 2024 18:13:38 +0100 Subject: [PATCH] process: abstract away create thread, exit thread and sigmask Previously we've opencoded calls to these functions, although they may be nicely abstracted with static inline functions. This change improves code readibility and portability. --- src/c/escape.d | 1 + src/c/process.d | 57 +++++++++++++----------------------------- src/c/threads/thread.d | 18 ++++++------- src/c/unixint.d | 32 ++++++++---------------- src/h/ecl.h | 3 --- src/h/internal.h | 28 +++++++++++++++------ src/h/stack-resize.h | 9 +++---- src/h/threads.h | 45 ++++++++++++++++++++++++++++++--- 8 files changed, 104 insertions(+), 89 deletions(-) diff --git a/src/c/escape.d b/src/c/escape.d index f9b12ecb6..4d9607100 100644 --- a/src/c/escape.d +++ b/src/c/escape.d @@ -175,6 +175,7 @@ ecl_thread_internal_error(const char *s) "Exitting thread.\n"); fflush(stderr); ecl_thread_exit(); + _ecl_unexpected_return(); } #endif diff --git a/src/c/process.d b/src/c/process.d index beaaaa16e..701fed10a 100644 --- a/src/c/process.d +++ b/src/c/process.d @@ -221,7 +221,7 @@ ecl_spawn_cpu(cl_object process) { cl_env_ptr the_env = ecl_process_env(); cl_env_ptr new_env = NULL; - int ok = 1; + int code = 0; /* Allocate and initialize the new cpu env. */ { new_env = _ecl_alloc_env(the_env); @@ -234,53 +234,30 @@ ecl_spawn_cpu(cl_object process) } /* Spawn the thread */ ecl_disable_interrupts_env(the_env); -#ifdef ECL_WINDOWS_THREADS +#if !defined(ECL_WINDOWS_THREADS) && defined(HAVE_SIGPROCMASK) { - HANDLE code; - DWORD threadId; - - code = (HANDLE)CreateThread(NULL, 0, thread_entry_point, new_env, 0, &threadId); - new_env->thread = code; - ok = code != NULL; + /* Block all asynchronous signals until the thread is completely set up. The + * synchronous signals SIGSEGV and SIGBUS are needed by the gc and and can't + * be blocked. */ + sigset_t new, previous; + sigfillset(&new); + sigdelset(&new, SIGSEGV); + sigdelset(&new, SIGBUS); + ecl_sigmask(SIG_BLOCK, &new, &previous); + code = ecl_thread_create(new_env, thread_entry_point); + ecl_sigmask(SIG_SETMASK, &previous, NULL); } -#else /* ECL_WINDOWS_THREADS */ - { - int code; - pthread_attr_t pthreadattr; - - pthread_attr_init(&pthreadattr); - pthread_attr_setdetachstate(&pthreadattr, PTHREAD_CREATE_DETACHED); - /* - * Block all asynchronous signals until the thread is completely - * set up. The synchronous signals SIGSEGV and SIGBUS are needed - * by the gc and thus can't be blocked. - */ -# ifdef HAVE_SIGPROCMASK - { - sigset_t new, previous; - sigfillset(&new); - sigdelset(&new, SIGSEGV); - sigdelset(&new, SIGBUS); - pthread_sigmask(SIG_BLOCK, &new, &previous); - code = pthread_create(&new_env->thread, &pthreadattr, - thread_entry_point, new_env); - pthread_sigmask(SIG_SETMASK, &previous, NULL); - } -# else - code = pthread_create(&new_env->thread, &pthreadattr, - thread_entry_point, new_env); -# endif - ok = (code == 0); - } -#endif /* ECL_WINDOWS_THREADS */ +#else + code = ecl_thread_create(new_env, thread_entry_point); +#endif /* Deal with the fallout of the thread creation. */ - if (!ok) { + if (code != 0) { process->process.env = NULL; ecl_modules_free_env(new_env); _ecl_dealloc_env(new_env); } ecl_enable_interrupts_env(the_env); - return ok ? new_env : NULL; + return code ? NULL : new_env; } #endif diff --git a/src/c/threads/thread.d b/src/c/threads/thread.d index 6f82d7ceb..3485ddfb0 100644 --- a/src/c/threads/thread.d +++ b/src/c/threads/thread.d @@ -90,7 +90,7 @@ run_process(cl_narg narg, ...) #ifdef HAVE_SIGPROCMASK { sigset_t *new = (sigset_t*)the_env->default_sigmask; - pthread_sigmask(SIG_SETMASK, new, NULL); + ecl_sigmask(SIG_SETMASK, new, NULL); } #endif process->process.phase = ECL_PROCESS_ACTIVE; @@ -124,7 +124,7 @@ run_process(cl_narg narg, ...) sigset_t new[1]; sigemptyset(new); sigaddset(new, ecl_option_values[ECL_OPT_THREAD_INTERRUPT_SIGNAL]); - pthread_sigmask(SIG_BLOCK, new, NULL); + ecl_sigmask(SIG_BLOCK, new, NULL); } #endif process->process.env = NULL; @@ -196,7 +196,7 @@ ecl_release_current_thread(void) sigset_t new[1]; sigemptyset(new); sigaddset(new, ecl_option_values[ECL_OPT_THREAD_INTERRUPT_SIGNAL]); - pthread_sigmask(SIG_BLOCK, new, NULL); + ecl_sigmask(SIG_BLOCK, new, NULL); } #endif process->process.phase = ECL_PROCESS_INACTIVE; @@ -424,8 +424,8 @@ mp_get_sigmask(void) sigset_t *mask_ptr = (sigset_t*)data->vector.self.b8; sigset_t no_signals; sigemptyset(&no_signals); - if (pthread_sigmask(SIG_BLOCK, &no_signals, mask_ptr)) - FElibc_error("MP:GET-SIGMASK failed in a call to pthread_sigmask", 0); + if (ecl_sigmask(SIG_BLOCK, &no_signals, mask_ptr)) + FElibc_error("MP:GET-SIGMASK failed in a call to ecl_sigmask", 0); @(return data); } @@ -433,8 +433,8 @@ static cl_object mp_set_sigmask(cl_object data) { sigset_t *mask_ptr = (sigset_t*)data->vector.self.b8; - if (pthread_sigmask(SIG_SETMASK, mask_ptr, NULL)) - FElibc_error("MP:SET-SIGMASK failed in a call to pthread_sigmask", 0); + if (ecl_sigmask(SIG_SETMASK, mask_ptr, NULL)) + FElibc_error("MP:SET-SIGMASK failed in a call to ecl_sigmask", 0); @(return data); } #endif @@ -455,8 +455,8 @@ mp_block_signals(void) * can thus never be blocked */ sigdelset(&all_signals, SIGSEGV); sigdelset(&all_signals, SIGBUS); - if (pthread_sigmask(SIG_SETMASK, &all_signals, NULL)) - FElibc_error("MP:BLOCK-SIGNALS failed in a call to pthread_sigmask",0); + if (ecl_sigmask(SIG_SETMASK, &all_signals, NULL)) + FElibc_error("MP:BLOCK-SIGNALS failed in a call to ecl_sigmask",0); @(return previous); #endif } diff --git a/src/c/unixint.d b/src/c/unixint.d index 2c61db18a..151fcf88a 100644 --- a/src/c/unixint.d +++ b/src/c/unixint.d @@ -41,7 +41,7 @@ * sections of code which are interruptible, and in which it is safe * for the handler to run arbitrary code, protect anything else. In * principle this "marking" can be done using POSIX functions such as - * pthread_sigmask() or sigprocmask(). + * pthread_sigmask() or sigprocmask() abstracted with ecl_sigmask(). * * However in practice this is slow, as it involves at least a * function call, resolving thread-local variables, etc, etc, and it @@ -307,11 +307,7 @@ unblock_signal(cl_env_ptr the_env, int signal) * We do not really "unblock" the signal, but rather restore * ECL's default sigmask. */ -# ifdef ECL_THREADS - pthread_sigmask(SIG_SETMASK, the_env->default_sigmask, NULL); -# else - sigprocmask(SIG_SETMASK, the_env->default_sigmask, NULL); -# endif + ecl_sigmask(SIG_SETMASK, the_env->default_sigmask, NULL); } #endif @@ -619,7 +615,7 @@ asynchronous_signal_servicing_thread() sigdelset(&handled_set, SIGSEGV); sigdelset(&handled_set, SIGBUS); } - pthread_sigmask(SIG_BLOCK, &handled_set, NULL); + ecl_sigmask(SIG_BLOCK, &handled_set, NULL); } /* * We create the object for communication. We need a lock to prevent other @@ -909,25 +905,25 @@ do_catch_signal(int code, cl_object action, cl_object process) return ECL_T; } else { sigset_t handled_set; - pthread_sigmask(SIG_SETMASK, NULL, &handled_set); + ecl_sigmask(SIG_SETMASK, NULL, &handled_set); if (action == @':mask') { sigaddset(&handled_set, code); } else { sigdelset(&handled_set, code); } - pthread_sigmask(SIG_SETMASK, &handled_set, NULL); + ecl_sigmask(SIG_SETMASK, &handled_set, NULL); return ECL_T; } # else { sigset_t handled_set; - sigprocmask(SIG_SETMASK, NULL, &handled_set); + ecl_sigmask(SIG_SETMASK, NULL, &handled_set); if (action == @':mask') { sigaddset(&handled_set, code); } else { sigdelset(&handled_set, code); } - sigprocmask(SIG_SETMASK, &handled_set, NULL); + ecl_sigmask(SIG_SETMASK, &handled_set, NULL); return ECL_T; } # endif /* !ECL_THREADS */ @@ -1304,11 +1300,7 @@ install_asynchronous_signal_handlers() #ifdef HAVE_SIGPROCMASK sigset_t *sigmask = ecl_core.first_env->default_sigmask = &main_thread_sigmask; ecl_core.default_sigmask_bytes = sizeof(sigset_t); -# ifdef ECL_THREADS - pthread_sigmask(SIG_SETMASK, NULL, sigmask); -# else - sigprocmask(SIG_SETMASK, NULL, sigmask); -# endif + ecl_sigmask(SIG_SETMASK, NULL, sigmask); #endif #if defined(ECL_THREADS) && defined(HAVE_SIGPROCMASK) ecl_mutex_init(&signal_thread_lock, TRUE); @@ -1319,11 +1311,7 @@ install_asynchronous_signal_handlers() } #endif #ifdef HAVE_SIGPROCMASK -# if defined(ECL_THREADS) - pthread_sigmask(SIG_SETMASK, sigmask, NULL); -# else - sigprocmask(SIG_SETMASK, sigmask, NULL); -# endif + ecl_sigmask(SIG_SETMASK, sigmask, NULL); #endif #ifdef ECL_WINDOWS_THREADS old_W32_exception_filter = @@ -1417,7 +1405,7 @@ install_synchronous_signal_handlers() mysignal(signal, process_interrupt_handler); #ifdef HAVE_SIGPROCMASK sigdelset(&main_thread_sigmask, signal); - pthread_sigmask(SIG_SETMASK, &main_thread_sigmask, NULL); + ecl_sigmask(SIG_SETMASK, &main_thread_sigmask, NULL); #endif } #endif diff --git a/src/h/ecl.h b/src/h/ecl.h index a95e27e70..80214bf5a 100644 --- a/src/h/ecl.h +++ b/src/h/ecl.h @@ -62,9 +62,6 @@ # include # endif # ifdef ECL_THREADS - typedef HANDLE pthread_t; - typedef HANDLE pthread_mutex_t; - typedef HANDLE pthread_cond_t; /*Dummy, not really used*/ # undef ERROR # ifdef GBC_BOEHM # define CreateThread GC_CreateThread diff --git a/src/h/internal.h b/src/h/internal.h index 615b46c0f..90d4b41c0 100755 --- a/src/h/internal.h +++ b/src/h/internal.h @@ -742,13 +742,6 @@ extern void ecl_cs_set_size(cl_env_ptr env, cl_index n); #ifdef ECL_THREADS extern ECL_API cl_object mp_suspend_loop(); extern ECL_API cl_object mp_break_suspend_loop(); - -# ifdef ECL_WINDOWS_THREADS -# define ecl_thread_exit() ExitThread(0); -# else -# define ecl_thread_exit() pthread_exit(NULL); -# endif /* ECL_WINDOWS_THREADS */ - #endif /* time.d */ @@ -918,6 +911,27 @@ extern void ecl_interrupt_process(cl_object process, cl_object function); #include +/* sigmask */ + +#ifdef HAVE_SIGPROCMASK +# include +# ifdef ECL_THREADS +static inline int +ecl_sigmask(int how, const sigset_t *set, sigset_t *oldset) +{ + return pthread_sigmask(how, set, oldset); +} +# else +static inline int +ecl_sigmask(int how, const sigset_t *set, sigset_t *oldset) +{ + return sigprocmask(how, set, oldset); +} +# endif +#endif + +/* global locks */ + #ifdef ECL_THREADS # define ECL_WITH_GLOBAL_LOCK_BEGIN(the_env) \ ECL_WITH_NATIVE_LOCK_BEGIN(the_env, &ecl_core.global_lock) diff --git a/src/h/stack-resize.h b/src/h/stack-resize.h index 93e9ee6bb..09e67df35 100644 --- a/src/h/stack-resize.h +++ b/src/h/stack-resize.h @@ -14,11 +14,10 @@ #ifndef ECL_STACK_RESIZE_H #define ECL_STACK_RESIZE_H -/* We can't block interrupts with ecl_disable_interrupts() and write - * in the thread local environment if we use fast interrupt dispatch - * via mprotect(), so we have to use sigprocmask instead. No - * performance problems, since this is only used for stack - * resizing. */ +/* We can't block interrupts with ecl_disable_interrupts() and write in the + * thread local environment if we use fast interrupt dispatch via mprotect(), so + * we have to use sigprocmask instead. No performance problems, since this is + * only used for stack resizing. */ #if defined(ECL_THREADS) && defined(ECL_USE_MPROTECT) # ifdef HAVE_SIGPROCMASK # include diff --git a/src/h/threads.h b/src/h/threads.h index 5571b228a..eb4ff6755 100644 --- a/src/h/threads.h +++ b/src/h/threads.h @@ -22,8 +22,8 @@ # endif #endif -#ifndef ECL_MUTEX_H -#define ECL_MUTEX_H +#ifndef ECL_THREADS_H +#define ECL_THREADS_H #include #ifdef ECL_WINDOWS_THREADS @@ -38,6 +38,45 @@ #endif #include +#ifdef ECL_WINDOWS_THREADS +/* Windows can't into typedefs in parameter lists. */ +/* typedef DWORD WINAPI (*ecl_thread_entry)(void *ptr); */ +static inline int +ecl_thread_create(cl_env_ptr the_env, /* ecl_thread_entry */ void* fun) +{ + HANDLE code; + DWORD threadId; + code = (HANDLE)CreateThread(NULL, 0, fun, the_env, 0, &threadId); + the_env->thread = code; + /* NULL handle is a failure. */ + return (code != NULL) ? 0 : 1; +} + +static inline void +ecl_thread_exit() +{ + ExitThread(0); +} +#else /* ECL_WINDOWS_THREADS */ +typedef void* (*ecl_thread_entry)(void *ptr); + +static inline int +ecl_thread_create(cl_env_ptr the_env, ecl_thread_entry fun) +{ + pthread_attr_t pthreadattr; + pthread_attr_init(&pthreadattr); + pthread_attr_setdetachstate(&pthreadattr, PTHREAD_CREATE_DETACHED); + return pthread_create(&the_env->thread, &pthreadattr, fun, the_env); +} + +static inline void +ecl_thread_exit() +{ + pthread_exit(NULL); +} +#endif /* ECL_WINDOWS_THREADS */ + + #if !defined(ECL_WINDOWS_THREADS) #define ECL_MUTEX_SUCCESS 0 @@ -734,6 +773,6 @@ ecl_rwlock_lock_write(ecl_rwlock_t *rwlock) #endif /* ECL_WINDOWS_THREADS */ -#endif /* ECL_MUTEX_H */ +#endif /* ECL_THREADS_H */ #endif /* ECL_THREADS */