Introduce READER_LOCK/UNLOCK macros

(refactoring)

Issue #473 (bdwgc).

* alloc.c (GC_get_disable_automatic_collection, GC_get_stop_func,
GC_get_start_callback, GC_get_on_collection_event): Use
READER_LOCK/UNLOCK() instead of exclusive LOCK/UNLOCK().
* dyn_load.c [DARWIN_DEBUG && !NO_DEBUGGING] (GC_dyld_image_add,
GC_dyld_image_remove): Likewise.
* finalize.c (GC_get_toggleref_func, GC_get_await_finalize_proc,
GC_get_interrupt_finalizers): Likewise.
* mark.c (GC_get_on_mark_stack_empty, GC_print_trace): Likewise.
* misc.c (GC_get_heap_usage_safe, GC_get_prof_stats, GC_get_warn_proc,
GC_get_abort_func, GC_dump, GC_get_memory_use, GC_get_oom_fn,
GC_get_on_heap_resize, GC_get_finalizer_notifier): Likewise.
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD]
(GC_is_thread_suspended): Likewise.
* pthread_support.c (GC_self_thread, GC_get_my_stackbottom,
GC_get_on_thread_event, GC_get_sp_corrector): Likewise.
* ptr_chck.c (GC_get_same_obj_print_proc,
GC_get_is_valid_displacement_print_proc, GC_get_is_visible_print_proc):
Likewise.
* headers.c (GC_next_block, GC_prev_block): Replace I_HOLD_LOCK() to
I_HOLD_READER_LOCK().
* mark_rts.c (GC_roots_present): Likewise.
* pthread_support.c (GC_count_threads, GC_segment_is_thread_stack,
GC_greatest_stack_base_below): Likewise.
* reclaim.c (GC_enumerate_reachable_objects_inner): Likewise.
* win32_threads.c [GC_PTHREADS] (GC_lookup_by_pthread): Likewise.
* include/gc/gc.h (GC_get_oom_fn, GC_get_on_heap_resize,
GC_get_on_collection_event, GC_get_on_thread_event,
GC_get_finalizer_notifier, GC_get_same_obj_print_proc,
GC_get_disable_automatic_collection, GC_get_stop_func,
GC_get_heap_usage_safe, GC_get_memory_use, GC_get_toggleref_func,
GC_get_await_finalize_proc, GC_get_interrupt_finalizers,
GC_get_warn_proc, GC_get_abort_func, GC_get_my_stackbottom, GC_dump):
Update comment that the allocator lock is aquired in the reader mode.
* include/gc/gc.h [GC_THREADS] (GC_thread_is_registered,
GC_get_sp_corrector): Likewise.
* include/gc/gc_inline.h (GC_print_free_list): Likewise.
* include/gc/gc_mark.h (GC_get_start_callback,
GC_get_on_mark_stack_empty): Likewise.
* include/gc/javaxfc.h [GC_THREADS] (GC_is_thread_suspended): Likewise.
* include/gc/gc.h (GC_get_heap_size, GC_REVEAL_POINTER): Update comment
that it is enough for the caller to hold the allocator lock in the
reader mode.
* include/gc/gc_mark.h (GC_iterate_free_hblks, GC_apply_to_all_blocks,
GC_is_black_listed, GC_is_marked,
GC_enumerate_reachable_objects_inner): Likewise.
* misc.c [THREADS] (GC_get_prof_stats_unsafe): Likewise.
* pthread_support.c (GC_lookup_thread): Likewise.
* reclaim.c (GC_print_free_list): Likewise.
* include/private/gc_locks.h: Mention I_HOLD_READER_LOCK() in comment
about I_[DONT_]HOLD_LOCK.
* include/private/gc_locks.h [!READER_LOCK] (READER_LOCK,
READER_UNLOCK): Define macro (redirect to [UN]LOCK).
* include/private/gc_locks.h [!READER_LOCK && GC_ASSERTIONS]
(I_HOLD_READER_LOCK): Define macro (redirect to I_HOLD_LOCK); add
comment.
* misc.c (GC_set_stackbottom): Use UNUSED_ARG() instead of cast to
void.
* pthread_support.c [GC_PTHREADS] (GC_pthread_join, GC_pthread_detach):
Use READER_LOCK/UNLOCK() instead of exclusive LOCK/UNLOCK() to call
GC_lookup_by_pthread().
* win32_threads.c [GC_PTHREADS] (GC_win32_cache_self_pthread): Add
assertion that the lock is held (in the exclusive mode).
* win32_threads.c (GC_get_next_stack): Likewise.

alloc.c

@@ -130,9 +130,9 @@ GC_API int GC_CALL GC_get_disable_automatic_collection(void)
 {
   int value;
 
-  LOCK();
+  READER_LOCK();
   value = (int)GC_disable_automatic_collection;
-  UNLOCK();
+  READER_UNLOCK();
   return value;
 }
 
@@ -236,9 +236,9 @@ GC_API GC_stop_func GC_CALL GC_get_stop_func(void)
 {
   GC_stop_func stop_func;
 
-  LOCK();
+  READER_LOCK();
   stop_func = GC_default_stop_func;
-  UNLOCK();
+  READER_UNLOCK();
   return stop_func;
 }
 
@@ -451,9 +451,9 @@ GC_API GC_start_callback_proc GC_CALL GC_get_start_callback(void)
 {
     GC_start_callback_proc fn;
 
-    LOCK();
+    READER_LOCK();
     fn = GC_start_call_back;
-    UNLOCK();
+    READER_UNLOCK();
     return fn;
 }
 
@@ -533,9 +533,9 @@ GC_API GC_on_collection_event_proc GC_CALL GC_get_on_collection_event(void)
 {
     GC_on_collection_event_proc fn;
 
-    LOCK();
+    READER_LOCK();
     fn = GC_on_collection_event;
-    UNLOCK();
+    READER_UNLOCK();
     return fn;
 }
 

dyn_load.c

@@ -1375,9 +1375,9 @@ STATIC void GC_dyld_image_add(const struct GC_MACH_HEADER *hdr,
   }
 
 # if defined(DARWIN_DEBUG) && !defined(NO_DEBUGGING)
-    LOCK();
+    READER_LOCK();
     GC_print_static_roots();
-    UNLOCK();
+    READER_UNLOCK();
 # endif
 }
 
@@ -1430,9 +1430,9 @@ STATIC void GC_dyld_image_remove(const struct GC_MACH_HEADER *hdr,
   }
 
 # if defined(DARWIN_DEBUG) && !defined(NO_DEBUGGING)
-    LOCK();
+    READER_LOCK();
     GC_print_static_roots();
-    UNLOCK();
+    READER_UNLOCK();
 # endif
 }
 

finalize.c

@@ -408,9 +408,9 @@ GC_INLINE void GC_complete_ongoing_collection(void) {
   {
     GC_toggleref_func fn;
 
-    LOCK();
+    READER_LOCK();
     fn = GC_toggleref_callback;
-    UNLOCK();
+    READER_UNLOCK();
     return fn;
   }
 
@@ -492,9 +492,9 @@ GC_API GC_await_finalize_proc GC_CALL GC_get_await_finalize_proc(void)
 {
   GC_await_finalize_proc fn;
 
-  LOCK();
+  READER_LOCK();
   fn = GC_object_finalized_proc;
-  UNLOCK();
+  READER_UNLOCK();
   return fn;
 }
 
@@ -1249,9 +1249,9 @@ GC_API unsigned GC_CALL GC_get_interrupt_finalizers(void)
 {
   unsigned value;
 
-  LOCK();
+  READER_LOCK();
   value = GC_interrupt_finalizers;
-  UNLOCK();
+  READER_UNLOCK();
   return value;
 }
 

headers.c

@@ -363,7 +363,7 @@ GC_INNER struct hblk * GC_next_block(struct hblk *h, GC_bool allow_free)
     REGISTER bottom_index * bi;
     REGISTER word j = ((word)h >> LOG_HBLKSIZE) & (BOTTOM_SZ-1);
 
-    GC_ASSERT(I_HOLD_LOCK());
+    GC_ASSERT(I_HOLD_READER_LOCK());
     GET_BI(h, bi);
     if (bi == GC_all_nils) {
         REGISTER word hi = (word)h >> (LOG_BOTTOM_SZ + LOG_HBLKSIZE);
@@ -398,7 +398,7 @@ GC_INNER struct hblk * GC_prev_block(struct hblk *h)
     bottom_index * bi;
     signed_word j = ((word)h >> LOG_HBLKSIZE) & (BOTTOM_SZ-1);
 
-    GC_ASSERT(I_HOLD_LOCK());
+    GC_ASSERT(I_HOLD_READER_LOCK());
     GET_BI(h, bi);
     if (bi == GC_all_nils) {
         word hi = (word)h >> (LOG_BOTTOM_SZ + LOG_HBLKSIZE);

include/gc/gc_inline.h

@@ -208,8 +208,8 @@ GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(1) void * GC_CALL
     } while (0)
 
 /* Print address of each object in the free list.  The caller should    */
-/* hold the allocator lock.  Defined only if the library has been       */
-/* compiled without NO_DEBUGGING.                                       */
+/* hold the allocator lock at least in the reader mode.  Defined only   */
+/* if the library has been compiled without NO_DEBUGGING.               */
 GC_API void GC_CALL GC_print_free_list(int /* kind */,
                                        size_t /* sz_in_granules */);
 

include/gc/gc_mark.h

@@ -207,7 +207,8 @@ typedef void (GC_CALLBACK * GC_walk_free_blk_fn)(struct GC_hblk_s *,
 
 /* Apply fn to each completely empty heap block.  It is the             */
 /* responsibility of the caller to avoid data race during the function  */
-/* execution (e.g. by acquiring the allocator lock).                    */
+/* execution (e.g. by acquiring the allocator lock at least in the      */
+/* reader mode).                                                        */
 GC_API void GC_CALL GC_iterate_free_hblks(GC_walk_free_blk_fn,
                                 GC_word /* client_data */) GC_ATTR_NONNULL(1);
 
@@ -216,15 +217,15 @@ typedef void (GC_CALLBACK * GC_walk_hblk_fn)(struct GC_hblk_s *,
 
 /* Apply fn to each allocated heap block.  It is the responsibility     */
 /* of the caller to avoid data race during the function execution (e.g. */
-/* by acquiring the allocator lock).                                    */
+/* by acquiring the allocator lock at least in the reader mode).        */
 GC_API void GC_CALL GC_apply_to_all_blocks(GC_walk_hblk_fn,
                                 GC_word /* client_data */) GC_ATTR_NONNULL(1);
 
 /* If there are likely to be false references to a block starting at h  */
 /* of the indicated length, then return the next plausible starting     */
 /* location for h that might avoid these false references.  Otherwise   */
-/* NULL is returned.  Assumes the allocator lock is held but no         */
-/* assertion about it by design.                                        */
+/* NULL is returned.  Assumes the allocator lock is held at least in    */
+/* the reader mode but no assertion about it by design.                 */
 GC_API struct GC_hblk_s *GC_CALL GC_is_black_listed(struct GC_hblk_s *,
                                                     GC_word /* len */);
 
@@ -347,15 +348,16 @@ GC_API void * GC_CALL GC_clear_stack(void *);
 /* potentially blocking calls.  In particular, it is not safe to        */
 /* allocate memory using the garbage collector from within the callback */
 /* function.  Both the setter and the getter acquire the allocator      */
-/* lock.                                                                */
+/* lock (in the reader mode in case of the getter).                     */
 typedef void (GC_CALLBACK * GC_start_callback_proc)(void);
 GC_API void GC_CALL GC_set_start_callback(GC_start_callback_proc);
 GC_API GC_start_callback_proc GC_CALL GC_get_start_callback(void);
 
 /* Slow/general mark bit manipulation.  The caller should hold the      */
-/* allocator lock.  GC_is_marked returns 1 (true) or 0.  The argument   */
-/* should be the real address of an object (i.e. the address of the     */
-/* debug header if there is one).                                       */
+/* allocator lock (the reader mode is enough in case of GC_is_marked).  */
+/* GC_is_marked returns 1 (true) or 0.  The argument should be the real */
+/* address of an object (i.e. the address of the debug header if there  */
+/* is one).                                                             */
 GC_API int GC_CALL GC_is_marked(const void *) GC_ATTR_NONNULL(1);
 GC_API void GC_CALL GC_clear_mark_bit(const void *) GC_ATTR_NONNULL(1);
 GC_API void GC_CALL GC_set_mark_bit(const void *) GC_ATTR_NONNULL(1);
@@ -379,9 +381,10 @@ GC_API void GC_CALL GC_set_push_other_roots(GC_push_other_roots_proc);
 GC_API GC_push_other_roots_proc GC_CALL GC_get_push_other_roots(void);
 
 /* Walk the GC heap visiting all reachable objects.  Assume the caller  */
-/* holds the allocator lock.  Object base pointer, object size and      */
-/* client custom data are passed to the callback (holding the allocator */
-/* lock).                                                               */
+/* holds the allocator lock at least in the reader mode.  Object base   */
+/* pointer, object size and client custom data are passed to the        */
+/* callback (holding the allocator lock in the same mode as the caller  */
+/* does).                                                               */
 typedef void (GC_CALLBACK * GC_reachable_object_proc)(void * /* obj */,
                                                 size_t /* bytes */,
                                                 void * /* client_data */);
@@ -399,7 +402,8 @@ GC_API void GC_CALL GC_print_trace_inner(GC_word /* gc_no */);
 /* Set the client for when mark stack is empty.  A client can use       */
 /* this callback to process (un)marked objects and push additional      */
 /* work onto the stack.  Useful for implementing ephemerons.  Both the  */
-/* setter and the getter acquire the allocator lock.                    */
+/* setter and the getter acquire the allocator lock (in the reader mode */
+/* in case of the getter).                                              */
 typedef struct GC_ms_entry * (GC_CALLBACK * GC_on_mark_stack_empty_proc)(
                                 struct GC_ms_entry * /* mark_stack_ptr */,
                                 struct GC_ms_entry * /* mark_stack_limit */);

include/gc/javaxfc.h

@@ -40,22 +40,26 @@
  *    finalizers which create new finalizable objects, though that's
  *    probably unlikely.
  * Thus this is not recommended for general use.
- * Acquire the allocator lock (to enqueue all finalizers).
+ * Acquires the allocator lock (to enqueue all finalizers).
  */
 GC_API void GC_CALL GC_finalize_all(void);
 
 #ifdef GC_THREADS
   /* External thread suspension support.  No thread suspension count    */
   /* (so a thread which has been suspended numerous times will be       */
   /* resumed with the very first call to GC_resume_thread).             */
-  /* Acquire the allocator lock.  Thread should be registered in GC     */
-  /* (otherwise no-op, GC_is_thread_suspended returns false).           */
+  /* Acquires the allocator lock.  Thread should be registered in GC.   */
   /* Unimplemented on some platforms.  Not recommended for general use. */
 # ifndef GC_SUSPEND_THREAD_ID
 #   define GC_SUSPEND_THREAD_ID void*
 # endif
   GC_API void GC_CALL GC_suspend_thread(GC_SUSPEND_THREAD_ID);
   GC_API void GC_CALL GC_resume_thread(GC_SUSPEND_THREAD_ID);
+
+  /* Is the given thread suspended externally?  The result is either    */
+  /* 1 (true) or 0.  Acquires the allocator lock in the reader mode.    */
+  /* Note: returns false if the thread is not registered in GC.         */
+  /* Unimplemented on some platforms (same as GC_suspend_thread).       */
   GC_API int GC_CALL GC_is_thread_suspended(GC_SUSPEND_THREAD_ID);
 #endif /* GC_THREADS */
 

include/private/gc_locks.h

@@ -22,13 +22,11 @@
 # error gc_locks.h should be included from gc_priv.h
 #endif
 
-/*
- * Mutual exclusion between allocator/collector routines.
- * Needed if there is more than one allocator thread.
- *
- * Note that I_HOLD_LOCK and I_DONT_HOLD_LOCK are used only positively
- * in assertions, and may return TRUE in the "don't know" case.
- */
+/* Mutual exclusion between allocator/collector routines.  Needed if    */
+/* there is more than one allocator thread.  Note that I_HOLD_LOCK,     */
+/* I_DONT_HOLD_LOCK and I_HOLD_READER_LOCK are used only positively in  */
+/* assertions, and may return TRUE in the "don't know" case.            */
+
 #ifdef THREADS
 
 # ifdef PCR
@@ -279,6 +277,16 @@
 # endif
 #endif
 
+#ifndef READER_LOCK
+# define READER_LOCK() LOCK()
+# define READER_UNLOCK() UNLOCK()
+# ifdef GC_ASSERTIONS
+    /* A macro to check that the allocator lock is held at least in the */
+    /* reader mode.                                                     */
+#   define I_HOLD_READER_LOCK() I_HOLD_LOCK()
+# endif
+#endif /* !READER_LOCK */
+
 # ifndef ENTER_GC
 #   define ENTER_GC()
 #   define EXIT_GC()

mark.c

@@ -354,9 +354,9 @@ GC_API GC_on_mark_stack_empty_proc GC_CALL GC_get_on_mark_stack_empty(void)
 {
   GC_on_mark_stack_empty_proc fn;
 
-  LOCK();
+  READER_LOCK();
   fn = GC_on_mark_stack_empty;
-  UNLOCK();
+  READER_UNLOCK();
   return fn;
 }
 
@@ -1582,9 +1582,9 @@ GC_API void GC_CALL GC_print_trace_inner(word gc_no)
 
 GC_API void GC_CALL GC_print_trace(word gc_no)
 {
-    LOCK();
+    READER_LOCK();
     GC_print_trace_inner(gc_no);
-    UNLOCK();
+    READER_UNLOCK();
 }
 
 #endif /* TRACE_BUF */

mark_rts.c

@@ -122,14 +122,14 @@ int GC_no_dls = 0;      /* Register dynamic library data segments.      */
     return ((val >> LOG_RT_SIZE) ^ val) & (RT_SIZE-1);
   }
 
-  /* Is a range starting at b already in the table? If so return a      */
+  /* Is a range starting at b already in the table? If so, return a     */
   /* pointer to it, else NULL.                                          */
   GC_INNER void * GC_roots_present(ptr_t b)
   {
     int h;
     struct roots *p;
 
-    GC_ASSERT(I_HOLD_LOCK());
+    GC_ASSERT(I_HOLD_READER_LOCK());
     h = rt_hash(b);
     for (p = GC_root_index[h]; p != NULL; p = p -> r_next) {
         if (p -> r_start == (ptr_t)b) break;