[pypy-commit] stmgc use-gcc: fix some issues with finalizers

[Raemi]

Author: Remi Meier <remi.meier at inf.ethz.ch>
Branch: use-gcc
Changeset: r1947:187d67c77fd4
Date: 2015-08-27 10:05 +0200
http://bitbucket.org/pypy/stmgc/changeset/187d67c77fd4/

Log:	fix some issues with finalizers

diff --git a/c8/stm/core.c b/c8/stm/core.c
--- a/c8/stm/core.c
+++ b/c8/stm/core.c
@@ -1365,9 +1365,10 @@
         stm_rewind_jmp_forget(STM_SEGMENT->running_thread);
     }
 
+    commit_finalizers();
+
     /* XXX do we still need a s_mutex_lock() section here? */
     s_mutex_lock();
-    commit_finalizers();
 
     /* update 'overflow_number' if needed */
     if (STM_PSEGMENT->overflow_number_has_been_used) {
diff --git a/c8/stm/finalizer.c b/c8/stm/finalizer.c
--- a/c8/stm/finalizer.c
+++ b/c8/stm/finalizer.c
@@ -29,6 +29,9 @@
 static void _commit_finalizers(void)
 {
     /* move finalizer lists to g_finalizers for major collections */
+    while (__sync_lock_test_and_set(&g_finalizers.lock, 1) != 0) {
+        spin_loop();
+    }
 
     if (STM_PSEGMENT->finalizers->run_finalizers != NULL) {
         /* copy 'STM_PSEGMENT->finalizers->run_finalizers' into
@@ -60,6 +63,8 @@
 
     free(STM_PSEGMENT->finalizers);
     STM_PSEGMENT->finalizers = NULL;
+
+    __sync_lock_release(&g_finalizers.lock);
 }
 
 static void abort_finalizers(struct stm_priv_segment_info_s *pseg)
@@ -389,7 +394,7 @@
 static void deal_with_objects_with_finalizers(void)
 {
     /* for non-light finalizers */
-
+    assert(_has_mutex());
     /* there is one 'objects_with_finalizers' list per segment.
        Objects that die at a major collection running in the same
        transaction as they were created will be put in the
@@ -495,23 +500,29 @@
 static void _invoke_general_finalizers(stm_thread_local_t *tl)
 {
     /* called between transactions */
-    static int lock = 0;
-
-    if (__sync_lock_test_and_set(&lock, 1) != 0) {
-        /* can't acquire the lock: someone else is likely already
-           running this function, so don't wait. */
-        return;
-    }
-
     rewind_jmp_buf rjbuf;
     stm_rewind_jmp_enterframe(tl, &rjbuf);
     _stm_start_transaction(tl);
+    /* XXX: become inevitable, bc. otherwise, we would need to keep
+       around the original g_finalizers.run_finalizers to restore it
+       in case of an abort. */
+    _stm_become_inevitable("finalizer-Tx");
 
-    fprintf(stderr, "run_finalizers: %lu\n", list_count(g_finalizers.run_finalizers));
-    _execute_finalizers(&g_finalizers);
+    while (__sync_lock_test_and_set(&g_finalizers.lock, 1) != 0) {
+        /* somebody is adding more finalizers (_commit_finalizer()) */
+        spin_loop();
+    }
+    struct finalizers_s copy = g_finalizers;
+    assert(copy.running_next == NULL);
+    g_finalizers.run_finalizers = NULL;
+    /* others may add to g_finalizers again: */
+    __sync_lock_release(&g_finalizers.lock);
+
+    fprintf(stderr, "run_finalizers: %lu\n", list_count(copy.run_finalizers));
+    _execute_finalizers(&copy);
 
     _stm_commit_transaction();
     stm_rewind_jmp_leaveframe(tl, &rjbuf);
 
-    __sync_lock_release(&lock);
+    LIST_FREE(copy.run_finalizers);
 }
diff --git a/c8/stm/finalizer.h b/c8/stm/finalizer.h
--- a/c8/stm/finalizer.h
+++ b/c8/stm/finalizer.h
@@ -1,5 +1,7 @@
 
+/* see deal_with_objects_with_finalizers() for explanation of these fields */
 struct finalizers_s {
+    long lock;
     struct list_s *objects_with_finalizers;
     uintptr_t count_non_young;
     struct list_s *run_finalizers;
diff --git a/c8/test/test_finalizer.py b/c8/test/test_finalizer.py
--- a/c8/test/test_finalizer.py
+++ b/c8/test/test_finalizer.py
@@ -273,59 +273,3 @@
         self.expect_finalized([])
         stm_major_collect()
         self.expect_finalized([lp1])
-
-
-class TestMoreRegularFinalizers(BaseTest):
-
-    def test_inevitable_in_finalizer(self):
-        lpo = stm_allocate_old(16)
-
-        self._first_time = True
-        @ffi.callback("void(object_t *)")
-        def finalizer(obj):
-            print "finalizing!", obj
-            stm_set_char(lpo, 'a')
-
-            if self._first_time:
-                self._first_time = False
-                # we will switch to the other TX and
-                # make it inevitable, so that our TX
-                # will abort on commit (or validate)
-                self.switch(0, validate=False)
-                self.become_inevitable()
-                self.switch(1, validate=False)
-
-        lib.stmcb_finalizer = finalizer
-        self._finalizer_keepalive = finalizer
-
-        # start a transaction with a finalizing obj
-        self.switch(1)
-        self.start_transaction()
-        lpf = stm_allocate_with_finalizer(16)
-
-        self.push_root(lpf)
-        stm_minor_collect()
-
-
-        self.switch(0)
-        self.start_transaction()
-        stm_set_char(lpo, 'x')
-        self.switch(1)
-        lpf = self.pop_root()
-        # commit this TX, start a new one, let lpf
-        # die with a major-gc:
-        self.commit_transaction()
-        self.start_transaction()
-        stm_major_collect()
-        # commit and run finalizer in separate TX
-        # that will abort because of a conflict
-        self.commit_transaction()
-
-        self.switch(0, validate=False)
-        assert stm_get_char(lpo) == 'x'
-        # commit the now-inevitable TX and run
-        # the aborted finalizer again
-        self.commit_transaction()
-        self.start_transaction()
-        # should now see the value set by finalizer
-        assert stm_get_char(lpo) == 'a'