Avoid potential data race during GC_dump execution

* include/gc.h (GC_dump): Separate declaration from GC_dump_named; add
comment that it acquires the lock.
* include/gc.h (GC_dump_named): Refine comment (add note that the lock
is not acquired by this function).
* include/private/gc_priv.h (GC_dump): Remove commented out declaration.
* include/private/gc_priv.h [!NO_DEBUGGING] (COND_DUMP): Call
GC_dump_named(NULL) instead of GC_dump().
* misc.c (GC_init) [GC_ASSERTIONS && GC_ALWAYS_MULTITHREADED]: Call
COND_DUMP while holding the allocation lock.
* misc.c [!NO_DEBUGGING] (GC_dump): Place LOCK/UNLOCK around
GC_dump_named() call.

diff --git a/include/gc.h b/include/gc.h
index c612848e5a37baa442e3b395b505a829e3815883..fbac4c3084bdcf209f9652110dedf836e50e3f5f 100644 (file)
--- a/include/gc.h
+++ b/include/gc.h
@@ -1566,12 +1566,17 @@ GC_API void * GC_CALL GC_is_valid_displacement(void * /* p */);
 /* Explicitly dump the GC state.  This is most often called from the    */
 /* debugger, or by setting the GC_DUMP_REGULARLY environment variable,  */
 /* but it may be useful to call it from client code during debugging.   */
-/* If name is specified (and non-NULL), it is printed to help           */
+/* The current collection number is printed in the header of the dump.  */
+/* Acquires the GC lock to avoid data races.                            */
+/* Defined only if the library has been compiled without NO_DEBUGGING.  */
+GC_API void GC_CALL GC_dump(void);
+
+/* The same as GC_dump but allows to specify the name of dump and does  */
+/* not acquire the lock.  If name is non-NULL, it is printed to help    */
 /* identifying individual dumps.  Otherwise the current collection      */
 /* number is used as the name.                                          */
 /* Defined only if the library has been compiled without NO_DEBUGGING.  */
 GC_API void GC_CALL GC_dump_named(const char * /* name */);
-GC_API void GC_CALL GC_dump(void); /* = GC_dump_named(NULL) */
 
 /* Dump information about each block of every GC memory section.        */
 /* Defined only if the library has been compiled without NO_DEBUGGING.  */

diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h
index 1d682a94c9714002a2ec5c38bcc9d6723921325b..5ac127a07f2c72c6b4ae8d57d7a0f52f01088d8e 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -2212,7 +2212,6 @@ void GC_print_block_list(void);
 void GC_print_hblkfreelist(void);
 void GC_print_heap_sects(void);
 void GC_print_static_roots(void);
-/* void GC_dump(void); - declared in gc.h */
 
 extern word GC_fo_entries; /* should be visible in extra/MacOS.c */
 
@@ -2524,8 +2523,9 @@ GC_INNER void *GC_store_debug_info_inner(void *p, word sz, const char *str,
 #ifndef NO_DEBUGGING
   GC_EXTERN GC_bool GC_dump_regularly;
                                 /* Generate regular debugging dumps.    */
-# define COND_DUMP if (EXPECT(GC_dump_regularly, FALSE)) GC_dump(); \
-                        else COND_DUMP_CHECKS
+# define COND_DUMP if (EXPECT(GC_dump_regularly, FALSE)) { \
+                        GC_dump_named(NULL); \
+                   } else COND_DUMP_CHECKS
 #else
 # define COND_DUMP COND_DUMP_CHECKS
 #endif

diff --git a/misc.c b/misc.c
index 55937ddc91e3fde03b6699fc80e5b45561a46cd5..2c4b6cee7c2afd26aba53c0de90d535c972d523e 100644 (file)
--- a/misc.c
+++ b/misc.c
@@ -871,6 +871,9 @@ GC_API void GC_CALL GC_init(void)
     /* LOCK(); -- no longer does anything this early. */
     word initial_heap_sz;
     IF_CANCEL(int cancel_state;)
+#   if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
+      DCL_LOCK_STATE;
+#   endif
 
     if (EXPECT(GC_is_initialized, TRUE)) return;
 #   ifdef REDIRECT_MALLOC
@@ -1281,31 +1284,30 @@ GC_API void GC_CALL GC_init(void)
       GC_pcr_install();
 #   endif
     GC_is_initialized = TRUE;
+#   if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
+        LOCK(); /* just to set GC_lock_holder */
+#   endif
 #   if defined(GC_PTHREADS) || defined(GC_WIN32_THREADS)
-#       if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
-          DCL_LOCK_STATE;
-          LOCK(); /* just to set GC_lock_holder */
-          GC_thr_init();
-          UNLOCK();
-#       else
-          GC_thr_init();
-#       endif
+        GC_thr_init();
 #       ifdef PARALLEL_MARK
           /* Actually start helper threads.     */
+#         if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
+            UNLOCK();
+#         endif
           GC_start_mark_threads_inner();
+#         if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
+            LOCK();
+#         endif
 #       endif
 #   endif
     COND_DUMP;
     /* Get black list set up and/or incremental GC started */
-      if (!GC_dont_precollect || GC_incremental) {
-#       if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
-          LOCK();
-          GC_gcollect_inner();
-          UNLOCK();
-#       else
-          GC_gcollect_inner();
-#       endif
-      }
+    if (!GC_dont_precollect || GC_incremental) {
+        GC_gcollect_inner();
+    }
+#   if defined(GC_ASSERTIONS) && defined(GC_ALWAYS_MULTITHREADED)
+        UNLOCK();
+#   endif
 #   ifdef STUBBORN_ALLOC
         GC_stubborn_init();
 #   endif
@@ -2190,7 +2192,11 @@ GC_API void * GC_CALL GC_do_blocking(GC_fn_type fn, void * client_data)
 #if !defined(NO_DEBUGGING)
   GC_API void GC_CALL GC_dump(void)
   {
+    DCL_LOCK_STATE;
+
+    LOCK();
     GC_dump_named(NULL);
+    UNLOCK();
   }
 
   GC_API void GC_CALL GC_dump_named(const char *name)