Fix lock assertion violation in GC_new_thread if GC_ALWAYS_MULTITHREADED

* include/private/gc_priv.h (GC_start_mark_threads_inner): Define macro
(or declare function depending on CAN_HANDLE_FORK).
* misc.c (GC_init): Surround GC_thr_init call with LOCK/UNLOCK (only
if GC_ASSERTIONS and GC_ALWAYS_MULTITHREADED otherwise redundant);
call GC_start_mark_threads_inner (if PARALLEL_MARK).
* pthread_support.c (GC_mark_thread): Update comment.
* win32_threads.c (GC_mark_thread): Likewise.
* pthread_support.c (start_mark_threads): Remove macro (moved to
gc_priv.h); rename function to GC_start_mark_threads_inner; replace
"static" to GC_INNER; check assertion on GC_fl_builder_count only if
the markers should actually be started; move the check for disabled
parallel markers (available_markers_m1) from GC_thr_init (make it
unconditional).
* win32_threads.c (start_mark_threads): Likewise.
* win32_threads.c (GC_start_mark_threads_inner): Add assertion about
the lock status.
* pthread_support.c (GC_thr_init): Remove comment about expected lock
status; add assertion about holding the lock (duplicating that in
GC_new_thread); remove start_mark_threads call (moved to GC_init).
* win32_threads.c (GC_thr_init): Likewise.

diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h
index 8f05bc1f6c0906e85d2fafccbd54b9a4496d428c..710c58c8bb94e56db60a7d51348c6150b775a34b 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -2418,6 +2418,12 @@ GC_INNER ptr_t GC_store_debug_info(ptr_t p, word sz, const char *str,
               /* my_mark_no.  Returns if the mark cycle finishes or     */
               /* was already done, or there was nothing to do for       */
               /* some other reason.                                     */
+
+# ifdef CAN_HANDLE_FORK
+#   define GC_start_mark_threads_inner GC_start_mark_threads
+# else
+    GC_INNER void GC_start_mark_threads_inner(void);
+# endif
 #endif /* PARALLEL_MARK */
 
 #if defined(GC_PTHREADS) && !defined(GC_WIN32_THREADS) && !defined(NACL) \

diff --git a/misc.c b/misc.c
index 29938c0ca6084bee8fa8ffdffd0d05cb3b1b4375..88885e3090e019f1c39f2e15582d187d6fe0ffae 100644 (file)
--- a/misc.c
+++ b/misc.c
@@ -1258,7 +1258,17 @@ GC_API void GC_CALL GC_init(void)
 #   endif
     GC_is_initialized = TRUE;
 #   if defined(GC_PTHREADS) || defined(GC_WIN32_THREADS)
-        GC_thr_init();
+#       if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
+          LOCK(); /* just to set GC_lock_holder */
+          GC_thr_init();
+          UNLOCK();
+#       else
+          GC_thr_init();
+#       endif
+#       ifdef PARALLEL_MARK
+          /* Actually start helper threads.     */
+          GC_start_mark_threads_inner();
+#       endif
 #   endif
     COND_DUMP;
     /* Get black list set up and/or incremental GC started */

diff --git a/pthread_support.c b/pthread_support.c
index 214dc2da230c86ee98c583f067449171d9bf57b1..7c5af41f5ba2db3ffcf2cd43679f7affa3856c9c 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -372,7 +372,7 @@ STATIC void * GC_mark_thread(void * id)
     marker_mach_threads[(word)id] = mach_thread_self();
 # endif
 
-  /* Inform start_mark_threads() about completion of marker data init.  */
+  /* Inform GC_start_mark_threads about completion of marker data init. */
   GC_acquire_mark_lock();
   if (0 == --GC_fl_builder_count)
     GC_notify_all_builder();
@@ -402,26 +402,25 @@ STATIC pthread_t GC_mark_threads[MAX_MARKERS];
 
 #ifdef CAN_HANDLE_FORK
   static int available_markers_m1 = 0;
-# define start_mark_threads GC_start_mark_threads
   GC_API void GC_CALL
 #else
 # define available_markers_m1 GC_markers_m1
-  static void
+  GC_INNER void
 #endif
-start_mark_threads(void)
+  GC_start_mark_threads_inner(void)
 {
     int i;
     pthread_attr_t attr;
 
     GC_ASSERT(I_DONT_HOLD_LOCK());
-    GC_ASSERT(GC_fl_builder_count == 0);
-#   ifdef CAN_HANDLE_FORK
-      if (available_markers_m1 <= 0 || GC_parallel) return;
+    if (available_markers_m1 <= 0) return;
                 /* Skip if parallel markers disabled or already started. */
+#   ifdef CAN_HANDLE_FORK
+      if (GC_parallel) return;
 #   endif
 
+    GC_ASSERT(GC_fl_builder_count == 0);
     INIT_REAL_SYMS(); /* for pthread_create */
-
     if (0 != pthread_attr_init(&attr)) ABORT("pthread_attr_init failed");
     if (0 != pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED))
         ABORT("pthread_attr_setdetachstate failed");
@@ -1072,9 +1071,9 @@ static void fork_child_proc(void)
   static void setup_mark_lock(void);
 #endif
 
-/* We hold the allocation lock. */
 GC_INNER void GC_thr_init(void)
 {
+  GC_ASSERT(I_HOLD_LOCK());
   if (GC_thr_initialized) return;
   GC_thr_initialized = TRUE;
 
@@ -1193,8 +1192,6 @@ GC_INNER void GC_thr_init(void)
       /* Disable true incremental collection, but generational is OK.   */
       GC_time_limit = GC_TIME_UNLIMITED;
       setup_mark_lock();
-      /* If we are using a parallel marker, actually start helper threads. */
-      start_mark_threads();
     }
 # endif
 }

diff --git a/win32_threads.c b/win32_threads.c
index d386f076df24373d736a3befbe1938dea68b7a89..53d0acfdf732b2a7700d74ae16ada86ae91dc201 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -1721,7 +1721,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       GC_marker_Id[(word)id] = GetCurrentThreadId();
 #   endif
 
-    /* Inform start_mark_threads() about completion of marker data init. */
+    /* Inform GC_start_mark_threads about completion of marker data init. */
     GC_acquire_mark_lock();
     if (0 == --GC_fl_builder_count)
       GC_notify_all_builder();
@@ -1760,28 +1760,28 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       /* Id not guaranteed to be unique. */
 #   endif
 
-    /* start_mark_threads is the same as in pthread_support.c except    */
+    /* GC_start_mark_threads is the same as in pthread_support.c except */
     /* for thread stack that is assumed to be large enough.             */
 #   ifdef CAN_HANDLE_FORK
       static int available_markers_m1 = 0;
-#     define start_mark_threads GC_start_mark_threads
       GC_API void GC_CALL
 #   else
-      static void
+      GC_INNER void
 #   endif
-    start_mark_threads(void)
+      GC_start_mark_threads_inner(void)
     {
       int i;
       pthread_attr_t attr;
       pthread_t new_thread;
 
       GC_ASSERT(I_DONT_HOLD_LOCK());
-      GC_ASSERT(GC_fl_builder_count == 0);
-#     ifdef CAN_HANDLE_FORK
-        if (available_markers_m1 <= 0 || GC_parallel) return;
+      if (available_markers_m1 <= 0) return;
                 /* Skip if parallel markers disabled or already started. */
+#     ifdef CAN_HANDLE_FORK
+        if (GC_parallel) return;
 #     endif
 
+      GC_ASSERT(GC_fl_builder_count == 0);
       if (0 != pthread_attr_init(&attr)) ABORT("pthread_attr_init failed");
       if (0 != pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED))
         ABORT("pthread_attr_setdetachstate failed");
@@ -1910,7 +1910,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
     static HANDLE builder_cv = (HANDLE)0; /* Event with manual reset.       */
     static HANDLE mark_cv = (HANDLE)0; /* Event with manual reset.          */
 
-    static void start_mark_threads(void)
+    GC_INNER void GC_start_mark_threads_inner(void)
     {
       int i;
 #     ifdef MSWINCE
@@ -1921,6 +1921,9 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
         unsigned thread_id;
 #     endif
 
+      GC_ASSERT(I_DONT_HOLD_LOCK());
+      if (available_markers_m1 <= 0) return;
+
       GC_ASSERT(GC_fl_builder_count == 0);
       /* Initialize GC_marker_cv[] fully before starting the    */
       /* first helper thread.                                   */
@@ -2342,7 +2345,6 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
 
 #endif /* GC_WINMAIN_REDIRECT */
 
-/* Called by GC_init() - we hold the allocation lock.   */
 GC_INNER void GC_thr_init(void)
 {
   struct GC_stack_base sb;
@@ -2454,16 +2456,6 @@ GC_INNER void GC_thr_init(void)
 
   GC_ASSERT(0 == GC_lookup_thread_inner(GC_main_thread));
   GC_register_my_thread_inner(&sb, GC_main_thread);
-
-# ifdef PARALLEL_MARK
-#   ifndef CAN_HANDLE_FORK
-      if (GC_parallel)
-#   endif
-    {
-      /* If we are using a parallel marker, actually start helper threads. */
-      start_mark_threads();
-    }
-# endif
 }
 
 #ifdef GC_PTHREADS