diff --git a/src/c/threads/process.d b/src/c/threads/process.d index 247741a07..d86284961 100644 --- a/src/c/threads/process.d +++ b/src/c/threads/process.d @@ -225,7 +225,7 @@ thread_entry_point(void *arg) * process.active = 2 * signals are disabled in the environment * - * Since process.active = 2, this process will not recevie + * Since process.active = 2, this process will not receive * signals that originate from other processes. Furthermore, * we expect not to get any other interrupts (SIGSEGV, SIGFPE) * if we do things right. @@ -237,18 +237,18 @@ thread_entry_point(void *arg) pthread_cleanup_push(thread_cleanup, (void *)process); #endif ecl_cs_set_org(env); - si_trap_fpe(@'last', Ct); ecl_list_process(process); - ecl_enable_interrupts_env(env); /* 2) Execute the code. The CATCH_ALL point is the destination * provides us with an elegant way to exit the thread: we just * do an unwind up to frs_top. */ - mp_get_lock_wait(process->process.exit_lock); - process->process.active = 1; - process->process.phase = ECL_PROCESS_ACTIVE; CL_CATCH_ALL_BEGIN(env) { + mp_get_lock_wait(process->process.exit_lock); + process->process.active = 1; + process->process.phase = ECL_PROCESS_ACTIVE; + ecl_enable_interrupts_env(env); + si_trap_fpe(@'last', Ct); ecl_bds_bind(env, @'mp::*current-process*', process); env->values[0] = cl_apply(2, process->process.function, process->process.args); @@ -260,6 +260,9 @@ thread_entry_point(void *arg) } process->process.exit_values = output; } + /* This will disable interrupts during the exit + * so that the unwinding is not interrupted. */ + process->process.phase = ECL_PROCESS_EXITING; ecl_bds_unwind1(env); } CL_CATCH_ALL_END; @@ -267,7 +270,6 @@ thread_entry_point(void *arg) * through this point. thread_cleanup is automatically invoked * marking the process as inactive. */ - process->process.phase = ECL_PROCESS_EXITING; #ifdef ECL_WINDOWS_THREADS thread_cleanup(process); return 1; @@ -427,9 +429,11 @@ mp_process_resume(cl_object process) cl_object mp_process_kill(cl_object process) { + assert_type_process(process); if (process->process.active && - process->process.phase != ECL_PROCESS_EXITING) + process->process.phase != ECL_PROCESS_EXITING) { return mp_interrupt_process(process, @'mp::exit-process'); + } } cl_object @@ -570,20 +574,21 @@ mp_process_whostate(cl_object process) cl_object mp_process_join(cl_object process) { + bool again = 1; assert_type_process(process); - /* We only wait for threads that we have created */ - if (process->process.active) { + /* We try to acquire a lock that is only owned by the process + * while it is active. */ + while (process->process.active && again) { cl_object l = process->process.exit_lock; - if (!Null(l)) { - while (process->process.active > 1) - cl_sleep(MAKE_FIXNUM(0)); - if (Null(mp_get_lock_wait(l))) { - FEerror("MP:PROCESS-JOIN: Error when " - "joining process ~A", - 1, process); - } - mp_giveup_lock(l); - } + if (Null(mp_get_lock_wait(l))) { + FEerror("MP:PROCESS-JOIN: Error when " + "joining process ~A", + 1, process); + } + /* Wait until process has moved past + * initialization phase */ + again = process->process.phase == ECL_PROCESS_BOOTING; + mp_giveup_lock(l); } return cl_values_list(process->process.exit_values); } diff --git a/src/c/threads/queue.d b/src/c/threads/queue.d index adf97f331..7fdaef03e 100644 --- a/src/c/threads/queue.d +++ b/src/c/threads/queue.d @@ -183,7 +183,7 @@ wakeup_this(cl_object p, int flags) { if (flags & ECL_WAKEUP_RESET_FLAG) p->process.waiting_for = Cnil; - mp_interrupt_process(p, Cnil); + ecl_interrupt_process(p, Cnil); } static void diff --git a/src/c/unixint.d b/src/c/unixint.d index ee8318136..d2cea00ff 100644 --- a/src/c/unixint.d +++ b/src/c/unixint.d @@ -236,6 +236,23 @@ mysignal(int code, void (*handler)(int, siginfo_t *, void*)) # define copy_siginfo(x,y) #endif +static bool +zombie_process(cl_env_ptr the_env) +{ + if (the_env == NULL) { + return 1; + } else { +#ifdef ECL_THREADS + /* When we are exiting a thread, we simply ignore all signals. */ + cl_object process = the_env->own_process; + return (!process->process.active || + process->process.phase == ECL_PROCESS_EXITING); +#else + return 0; +#endif + } +} + static bool interrupts_disabled_by_C(cl_env_ptr the_env) { @@ -256,7 +273,7 @@ handler_fn_protype(lisp_signal_handler, int sig, siginfo_t *info, void *aux) { cl_env_ptr the_env = ecl_process_env(); /* The lisp environment might not be installed. */ - if (the_env == NULL) + if (zombie_process(the_env)) return Cnil; #if defined(ECL_THREADS) && !defined(ECL_MS_WINDOWS_HOST) if (sig == ecl_get_option(ECL_OPT_THREAD_INTERRUPT_SIGNAL)) { @@ -465,15 +482,8 @@ handle_or_queue(cl_object signal_code, int code) if (Null(signal_code) || signal_code == NULL) return; the_env = ecl_process_env(); -#ifdef ECL_THREADS - /* When we are exiting a thread, we simply ingnore all signals. */ - { - cl_object process = the_env->own_process; - if (!process->process.active || - process->process.phase == ECL_PROCESS_EXITING) - return; - } -#endif + if (zombie_process(the_env)) + return; /* * If interrupts are disabled by lisp we are not so eager on * detecting when the interrupts become enabled again. We @@ -558,8 +568,8 @@ handler_fn_protype(sigsegv_handler, int sig, siginfo_t *info, void *aux) } the_env = ecl_process_env(); /* The lisp environment might not be installed. */ - if (the_env == NULL) - return; + if (zombie_process(the_env)) + return; #if defined(SA_SIGINFO) && defined(ECL_USE_MPROTECT) /* We access the environment when it was protected. That * means there was a pending signal. */ @@ -622,7 +632,7 @@ handler_fn_protype(sigbus_handler, int sig, siginfo_t *info, void *aux) reinstall_signal(sig, sigsegv_handler); the_env = ecl_process_env(); /* The lisp environment might not be installed. */ - if (the_env == NULL) + if (zombie_process(the_env)) return; #if defined(SA_SIGINFO) && defined(ECL_USE_MPROTECT) /* We access the environment when it was protected. That @@ -827,26 +837,29 @@ void ecl_interrupt_process(cl_object process, cl_object function) { /* - * We first get the lock to ensure that we do not interrupt - * the thread while acquiring the queue lock. Then the code - * takes two different approaches depending on the platform: - * + * We first ensure that the process is active and running + * and past the initialization phase, where it has set up + * the environment. Then: * - In Windows it sets up a trap in the stack, so that the * uncaught exception handler can catch it and process it. * - In POSIX systems it sends a user level interrupt to * the thread, which then decides how to act. */ - unlikely_if (!process->process.active) { - FEerror("Cannot interrupt non-active process ~A", - 1, process); + /* Trivial spinlock to ensure that the process is past + * the section where it establishes a CATCH */ + while (process->process.phase == ECL_PROCESS_BOOTING) + ecl_musleep(0.0, 0); + /* And then only care when the process is really active, for + * otherwise the signal will be ignored. */ + if (process->process.phase == ECL_PROCESS_ACTIVE) { + /* If FUNCTION is NIL, we just intend to wake up the + * process from some call to ecl_musleep() */ + if (!Null(function)) { + function = si_coerce_to_function(function); + queue_signal(process->process.env, function); + } + do_interrupt_thread(process); } - /* If FUNCTION is NIL, we just intend to wake up the - * process from some call to ecl_musleep() */ - if (!Null(function)) { - function = si_coerce_to_function(function); - queue_signal(process->process.env, function); - } - do_interrupt_thread(process); } #endif /* ECL_THREADS */