Cleanup and fix for FinalizationService

PullRequest: truffleruby/489

Author: eregon

src/main/java/org/truffleruby/core/FinalizationService.java

@@ -60,11 +60,11 @@ public static class FinalizerReference extends PhantomReference<Object> {
          * {@link FinalizationService} monitor, to avoid concurrent access.
          */
         private final Deque<Finalizer> finalizers = new LinkedList<>();
+
+        /** The doubly-linked list of FinalizerReference, needed to collect finalizer Procs for ObjectSpace. */
         private FinalizerReference next = null;
         private FinalizerReference prev = null;
 
-        private static FinalizerReference first = null;
-
         private FinalizerReference(Object object, ReferenceQueue<? super Object> queue) {
             super(object, queue);
         }
@@ -73,10 +73,11 @@ private void addFinalizer(Class<?> owner, Runnable action, DynamicObject root) {
             finalizers.addLast(new Finalizer(owner, action, root));
         }
 
-        private FinalizerReference removeFinalizers(Class<?> owner) {
+        private FinalizerReference removeFinalizers(FinalizationService finalizationService, Class<?> owner) {
             finalizers.removeIf(f -> f.getOwner() == owner);
+
             if (finalizers.isEmpty()) {
-                remove(this);
+                finalizationService.remove(this);
                 return null;
             } else {
                 return this;
@@ -98,24 +99,21 @@ private void collectRoots(Collection<DynamicObject> roots) {
     }
 
     private final RubyContext context;
-
     private final ReferenceQueue<Object> finalizerQueue = new ReferenceQueue<>();
-
+    /** The finalizer Ruby thread, spawned lazily. */
     private DynamicObject finalizerThread;
+    /** The head of a doubly-linked list of FinalizerReference, needed to collect finalizer Procs for ObjectSpace. */
+    private FinalizerReference first = null;
 
     public FinalizationService(RubyContext context) {
         this.context = context;
     }
 
-    public FinalizerReference addFinalizer(Object object, FinalizerReference ref, Class<?> owner, Runnable action) {
-        return addFinalizer(object, ref, owner, action, null);
-    }
-
     public synchronized FinalizerReference addFinalizer(Object object, FinalizerReference finalizerReference, Class<?> owner, Runnable action, DynamicObject root) {
 
         if (finalizerReference == null) {
             finalizerReference = new FinalizerReference(object, finalizerQueue);
-            FinalizationService.add(finalizerReference);
+            add(finalizerReference);
         }
 
         finalizerReference.addFinalizer(owner, action, root);
@@ -163,17 +161,18 @@ private void createFinalizationThread() {
 
         threadManager.initialize(finalizerThread, null, "finalizer", () -> {
             while (true) {
-                final FinalizerReference finalizerReference = (FinalizerReference) threadManager.runUntilResult(null,
-                        finalizerQueue::remove);
+                final FinalizerReference finalizerReference =
+                        (FinalizerReference) threadManager.runUntilResult(null, finalizerQueue::remove);
 
                 runFinalizer(finalizerReference);
             }
         });
     }
 
     private void runFinalizer(FinalizerReference finalizerReference) {
+        remove(finalizerReference);
+
         try {
-            FinalizationService.remove(finalizerReference);
             while (!context.isFinalizing()) {
                 final Finalizer finalizer;
                 synchronized (this) {
@@ -198,7 +197,7 @@ private void runFinalizer(FinalizerReference finalizerReference) {
     }
 
     public synchronized void collectRoots(Collection<DynamicObject> roots) {
-        FinalizerReference finalizerReference = FinalizerReference.first;
+        FinalizerReference finalizerReference = first;
         while (finalizerReference != null) {
             finalizerReference.collectRoots(roots);
             finalizerReference = finalizerReference.next;
@@ -207,23 +206,24 @@ public synchronized void collectRoots(Collection<DynamicObject> roots) {
 
     public synchronized FinalizerReference removeFinalizers(Object object, FinalizerReference ref, Class<?> owner) {
         if (ref != null) {
-            return ref.removeFinalizers(owner);
+            return ref.removeFinalizers(this, owner);
         } else {
             return null;
         }
     }
 
-    static synchronized void remove(FinalizerReference ref) {
+    private synchronized void remove(FinalizerReference ref) {
         if (ref.next == ref) {
             // Already removed.
             return;
         }
 
-        if (FinalizerReference.first == ref) {
+        if (first == ref) {
             if (ref.next != null) {
-                FinalizerReference.first = ref.next;
+                first = ref.next;
             } else {
-                FinalizerReference.first = ref.prev;
+                // The list becomes empty
+                first = null;
             }
         }
 
@@ -239,12 +239,12 @@ static synchronized void remove(FinalizerReference ref) {
         ref.prev = ref;
     }
 
-    static synchronized void add(FinalizerReference newRef) {
-        if (FinalizerReference.first != null) {
-            newRef.next = FinalizerReference.first;
-            FinalizerReference.first.prev = newRef;
+    private synchronized void add(FinalizerReference newRef) {
+        if (first != null) {
+            newRef.next = first;
+            first.prev = newRef;
         }
-        FinalizerReference.first = newRef;
+        first = newRef;
     }
 
 }

src/main/java/org/truffleruby/extra/ffi/Pointer.java

@@ -248,7 +248,7 @@ public synchronized void enableAutorelease(FinalizationService finalizationServi
 
         // We must be careful here that the finalizer does not capture the Pointer itself that we'd
         // like to finalize.
-        finalizerRef = finalizationService.addFinalizer(this, finalizerRef, Pointer.class, new FreeAddressFinalizer(address));
+        finalizerRef = finalizationService.addFinalizer(this, finalizerRef, Pointer.class, new FreeAddressFinalizer(address), null);
 
         autorelease = true;
     }