diff options
author | Ivan Maidanski <ivmai@mail.ru> | 2015-10-21 00:48:56 +0300 |
---|---|---|
committer | Ivan Maidanski <ivmai@mail.ru> | 2015-10-21 00:48:56 +0300 |
commit | 2d79f88938ca8bbe2eabf05dc4ba98fd0ec63dae (patch) | |
tree | 9b8b8820cf32b1ae291c894d383b8ee5b14ef65f | |
parent | 60af658b0071b5ed3c9b8e6c0ad6ac3adc086a3e (diff) | |
download | bdwgc-2d79f88938ca8bbe2eabf05dc4ba98fd0ec63dae.tar.gz |
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.
-rw-r--r-- | include/private/gc_priv.h | 6 | ||||
-rw-r--r-- | misc.c | 12 | ||||
-rw-r--r-- | pthread_support.c | 19 | ||||
-rw-r--r-- | win32_threads.c | 32 |
4 files changed, 37 insertions, 32 deletions
diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 8f05bc1f..710c58c8 100644 --- 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) \ @@ -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 214dc2da..7c5af41f 100644 --- 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 d386f076..53d0acfd 100644 --- 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 |