Fix race in GC_print_all_errors regarding GC_leaked
authorIvan Maidanski <ivmai@mail.ru>
Mon, 16 Sep 2013 04:50:49 +0000 (08:50 +0400)
committerIvan Maidanski <ivmai@mail.ru>
Mon, 16 Sep 2013 21:46:55 +0000 (01:46 +0400)
* reclaim.c (GC_add_leaked): Remove FIXME.
* reclaim.c (GC_print_all_errors): Declare n_leaked, leaked[] local
variables initialized from GC_[n_]leaked while holding the allocation
lock (reset GC_n_leaked and GC_leaked[] as well); add GC_ASSERT for
n_leaked; use [n_]leaked while printing leaked objects count and
pointers.
* reclaim.c (GC_print_all_errors): Acquire lock to reset
printing_errors.

reclaim.c

index 285e146..a332ef6 100644 (file)
--- a/reclaim.c
+++ b/reclaim.c
@@ -57,7 +57,6 @@ GC_INLINE void GC_add_leaked(ptr_t leaked)
 #  endif
 
     GC_have_errors = TRUE;
-    /* FIXME: Prevent adding an object while printing leaked ones.      */
     if (GC_n_leaked < MAX_LEAKED) {
       GC_leaked[GC_n_leaked++] = leaked;
       /* Make sure it's not reclaimed this cycle */
@@ -71,7 +70,8 @@ GC_INNER void GC_print_all_errors(void)
 {
     static GC_bool printing_errors = FALSE;
     GC_bool have_errors;
-    unsigned i;
+    unsigned i, n_leaked;
+    ptr_t leaked[MAX_LEAKED];
     DCL_LOCK_STATE;
 
     LOCK();
@@ -81,6 +81,11 @@ GC_INNER void GC_print_all_errors(void)
     }
     have_errors = GC_have_errors;
     printing_errors = TRUE;
+    n_leaked = GC_n_leaked;
+    GC_ASSERT(n_leaked <= MAX_LEAKED);
+    BCOPY(GC_leaked, leaked, n_leaked * sizeof(ptr_t));
+    GC_n_leaked = 0;
+    BZERO(GC_leaked, n_leaked * sizeof(ptr_t));
     UNLOCK();
 
     if (GC_debugging_started) {
@@ -89,17 +94,15 @@ GC_INNER void GC_print_all_errors(void)
       have_errors = FALSE;
     }
 
-    if (GC_n_leaked > 0) {
-        GC_err_printf("Found %u leaked objects:\n", GC_n_leaked);
+    if (n_leaked > 0) {
+        GC_err_printf("Found %u leaked objects:\n", n_leaked);
         have_errors = TRUE;
     }
-    for (i = 0; i < GC_n_leaked; ++i) {
-        ptr_t p = GC_leaked[i];
+    for (i = 0; i < n_leaked; i++) {
+        ptr_t p = leaked[i];
         GC_print_heap_obj(p);
         GC_free(p);
-        GC_leaked[i] = 0;
     }
-    GC_n_leaked = 0;
 
     if (have_errors
 #       ifndef GC_ABORT_ON_LEAK
@@ -109,7 +112,9 @@ GC_INNER void GC_print_all_errors(void)
       ABORT("Leaked or smashed objects encountered");
     }
 
+    LOCK();
     printing_errors = FALSE;
+    UNLOCK();
 }