From: Ivan Maidanski <ivmai@mail.ru>
Date: Fri, 3 Nov 2017 08:03:04 +0000 (+0300)
Subject: Avoid data race in finalized_count (gctest)
X-Git-Tag: v7.4.8~15
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0c7fdf4d9d5993337b7449528f84f8490bf30e2e;p=gc

Avoid data race in finalized_count (gctest)
(Cherry-pick commit ffc89ea1 from 'release-7_6' branch.)

* tests/test.c (FINALIZER_LOCK, FINALIZER_UNLOCK): New macro.
* tests/test.c [GC_PTHREADS] (incr_lock): Move static variable out
of finalizer() and mktree() (use a single lock variable to access
finalized_count in finalizer, mktree and tree_test).
* tests/test.c (dropped_something): Change type back to int; remove
volatile.
* tests/test.c (finalizer, mktree): Use FINALIZER_[UN]LOCK() instead
of PCR_ThCrSec_Enter/ExitSys(), pthread_mutex_[un]lock(),
Enter/LeaveCriticalSection().
* tests/test.c (tree_test): Use FINALIZER_[UN]LOCK() to avoid data
race when accessing finalized_count and dropped_something; do not use
AO_load() and AO_store() to access dropped_something.
---

diff --git a/tests/test.c b/tests/test.c
index 0a51786e..940aeadc 100644
--- a/tests/test.c
+++ b/tests/test.c
@@ -116,11 +116,23 @@
 #   define INIT_FORK_SUPPORT /* empty */
 # endif
 
-# if defined(GC_WIN32_THREADS) && !defined(GC_PTHREADS)
-    static CRITICAL_SECTION incr_cs;
-# endif
+#ifdef PCR
+# define FINALIZER_LOCK() PCR_ThCrSec_EnterSys()
+# define FINALIZER_UNLOCK() PCR_ThCrSec_ExitSys()
+#elif defined(GC_PTHREADS)
+  static pthread_mutex_t incr_lock = PTHREAD_MUTEX_INITIALIZER;
+# define FINALIZER_LOCK() pthread_mutex_lock(&incr_lock)
+# define FINALIZER_UNLOCK() pthread_mutex_unlock(&incr_lock)
+#elif defined(GC_WIN32_THREADS)
+  static CRITICAL_SECTION incr_cs;
+# define FINALIZER_LOCK() EnterCriticalSection(&incr_cs)
+# define FINALIZER_UNLOCK() LeaveCriticalSection(&incr_cs)
+#else
+# define FINALIZER_LOCK() (void)0
+# define FINALIZER_UNLOCK() (void)0
+#endif /* !THREADS */
 
-# include <stdarg.h>
+#include <stdarg.h>
 
 #define CHECK_GCLIB_VERSION \
             if (GC_get_version() != ((GC_VERSION_MAJOR<<16) \
@@ -732,35 +744,20 @@ typedef struct treenode {
 
 int finalizable_count = 0;
 int finalized_count = 0;
-volatile int dropped_something = 0;
+int dropped_something = 0;
 
 void GC_CALLBACK finalizer(void * obj, void * client_data)
 {
   tn * t = (tn *)obj;
 
-# ifdef PCR
-     PCR_ThCrSec_EnterSys();
-# endif
-# if defined(GC_PTHREADS)
-    static pthread_mutex_t incr_lock = PTHREAD_MUTEX_INITIALIZER;
-    pthread_mutex_lock(&incr_lock);
-# elif defined(GC_WIN32_THREADS)
-    EnterCriticalSection(&incr_cs);
-# endif
+  FINALIZER_LOCK();
   if ((int)(GC_word)client_data != t -> level) {
      GC_printf("Wrong finalization data - collector is broken\n");
      FAIL;
   }
   finalized_count++;
   t -> level = -1;      /* detect duplicate finalization immediately */
-# ifdef PCR
-    PCR_ThCrSec_ExitSys();
-# endif
-# if defined(GC_PTHREADS)
-    pthread_mutex_unlock(&incr_lock);
-# elif defined(GC_WIN32_THREADS)
-    LeaveCriticalSection(&incr_cs);
-# endif
+  FINALIZER_UNLOCK();
 }
 
 size_t counter = 0;
@@ -816,28 +813,13 @@ tn * mktree(int n)
 #       endif
 
         {
-#         ifdef PCR
-            PCR_ThCrSec_EnterSys();
-#         endif
-#         if defined(GC_PTHREADS)
-            static pthread_mutex_t incr_lock = PTHREAD_MUTEX_INITIALIZER;
-            pthread_mutex_lock(&incr_lock);
-#         elif defined(GC_WIN32_THREADS)
-            EnterCriticalSection(&incr_cs);
-#         endif
+          FINALIZER_LOCK();
                 /* Losing a count here causes erroneous report of failure. */
           finalizable_count++;
 #         ifndef GC_NO_FINALIZATION
             my_index = live_indicators_count++;
 #         endif
-#         ifdef PCR
-            PCR_ThCrSec_ExitSys();
-#         endif
-#         if defined(GC_PTHREADS)
-            pthread_mutex_unlock(&incr_lock);
-#         elif defined(GC_WIN32_THREADS)
-            LeaveCriticalSection(&incr_cs);
-#         endif
+          FINALIZER_UNLOCK();
         }
 
 #     ifndef GC_NO_FINALIZATION
@@ -1012,11 +994,13 @@ void tree_test(void)
       alloc_small(5000000);
 #   endif
     chktree(root, TREE_HEIGHT);
-    if (finalized_count && ! dropped_something) {
+    FINALIZER_LOCK();
+    if (finalized_count && !dropped_something) {
         GC_printf("Premature finalization - collector is broken\n");
         FAIL;
     }
     dropped_something = 1;
+    FINALIZER_UNLOCK();
     GC_noop1((word)root);       /* Root needs to remain live until      */
                                 /* dropped_something is set.            */
     root = mktree(TREE_HEIGHT);