From 42fdbba968c0494887e1255f8877cd0bca784603 Mon Sep 17 00:00:00 2001
From: Marius Gerbershagen <marius.gerbershagen@gmail.com>
Date: Sun, 12 Apr 2020 20:34:23 +0200
Subject: [PATCH] process.d: get start_stop_spinlock in thread_cleanup instead
 of thread_entry_point

Getting this lock in thread_entry_point was problematic: when the
thread was killed from another thread the catch point in
thread_entry_point was reached and the call to ecl_get_spinlock was
skipped. This lead to threads exiting without protection and to
segfaults.
---
 src/c/threads/process.d | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/c/threads/process.d b/src/c/threads/process.d
index eff9c1caa..be785bbec 100755
--- a/src/c/threads/process.d
+++ b/src/c/threads/process.d
@@ -187,13 +187,14 @@ thread_cleanup(void *aux)
    * executed, never use pthread_cancel() to kill a process, but
    * rather use the lisp functions mp_interrupt_process() and
    * mp_process_kill().
-   *
-   * NOTE: to avoid race conditions, this method must be executed
-   * while process.start_stop_spinlock is held.
    */
   cl_object process = (cl_object)aux;
   cl_env_ptr env = process->process.env;
+
   /* The following flags will disable all interrupts. */
+  if (env) {
+    ecl_get_spinlock(env, &process->process.start_stop_spinlock);
+  }
   AO_store_full((AO_t*)&process->process.phase, ECL_PROCESS_EXITING);
   if (env) {
     ecl_clear_bignum_registers(env);
@@ -216,7 +217,9 @@ thread_cleanup(void *aux)
   mp_barrier_unblock(3, process->process.exit_barrier, @':disable', ECL_T);
   ecl_set_process_env(NULL);
   if (env) _ecl_dealloc_env(env);
+
   AO_store_release((AO_t*)&process->process.phase, ECL_PROCESS_INACTIVE);
+  ecl_giveup_spinlock(&process->process.start_stop_spinlock);
 }
 
 #ifdef ECL_WINDOWS_THREADS
@@ -282,9 +285,6 @@ static DWORD WINAPI thread_entry_point(void *arg)
       /* ABORT restart. */
       process->process.exit_values = args;
     } ECL_RESTART_CASE_END;
-    /* This will disable interrupts during the exit
-     * so that the unwinding is not interrupted. */
-    ecl_get_spinlock(env, &process->process.start_stop_spinlock);
     process->process.phase = ECL_PROCESS_EXITING;
     ecl_bds_unwind1(env);
   } ECL_CATCH_ALL_END;
@@ -295,13 +295,9 @@ static DWORD WINAPI thread_entry_point(void *arg)
    */
 #ifdef ECL_WINDOWS_THREADS
   thread_cleanup(process);
-#else
-  pthread_cleanup_pop(1);
-#endif
-  ecl_giveup_spinlock(&process->process.start_stop_spinlock);
-#ifdef ECL_WINDOWS_THREADS
   return 1;
 #else
+  pthread_cleanup_pop(1);
   return NULL;
 #endif
 }
@@ -436,9 +432,7 @@ ecl_release_current_thread(void)
 
   int cleanup = env->cleanup;
   cl_object own_process = env->own_process;
-  ecl_get_spinlock(env, &own_process->process.start_stop_spinlock);
   thread_cleanup(own_process);
-  ecl_giveup_spinlock(&own_process->process.start_stop_spinlock);
 #ifdef GBC_BOEHM
   if (cleanup) {
     GC_unregister_my_thread();