Thread-safety improvements

jwharm · Jun 28, 2023 · ee69e0d · ee69e0d
1 parent afb0f82
commit ee69e0d
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 42 deletions.
diff --git a/glib/main/java/io/github/jwharm/javagi/interop/InstanceCache.java b/glib/main/java/io/github/jwharm/javagi/interop/InstanceCache.java
@@ -3,8 +3,8 @@
 import java.lang.foreign.MemorySegment;
 import java.lang.ref.Cleaner;
 import java.lang.ref.WeakReference;
-import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 
 import io.github.jwharm.javagi.types.TypeCache;
@@ -21,8 +21,8 @@
  */
 public class InstanceCache {
 
-    private final static Map<MemorySegment, Proxy> strongReferences = new HashMap<>();
-    private final static Map<MemorySegment, WeakReference<Proxy>> weakReferences = new HashMap<>();
+    private final static Map<MemorySegment, Proxy> strongReferences = new ConcurrentHashMap<>();
+    private final static Map<MemorySegment, WeakReference<Proxy>> weakReferences = new ConcurrentHashMap<>();
     private static final Cleaner CLEANER = Cleaner.create();
 
     /**

diff --git a/glib/main/java/io/github/jwharm/javagi/interop/MemoryCleaner.java b/glib/main/java/io/github/jwharm/javagi/interop/MemoryCleaner.java
@@ -43,12 +43,14 @@ public static void register(Proxy proxy) {
 
         // Put the address in the cache (or increase the refcount)
         MemorySegment address = proxy.handle();
-        Cached cached = references.get(address);
-        if (cached == null) {
-            references.put(address, new Cached(false, 1, null));
-            CLEANER.register(proxy, new StructFinalizer(address));
-        } else {
-            references.put(address, new Cached(false, cached.references + 1, cached.freeFunc));
+        synchronized (references) {
+            Cached cached = references.get(address);
+            if (cached == null) {
+                references.put(address, new Cached(false, 1, null));
+                CLEANER.register(proxy, new StructFinalizer(address));
+            } else {
+                references.put(address, new Cached(false, cached.references + 1, cached.freeFunc));
+            }
         }
     }
 
@@ -59,8 +61,10 @@ public static void register(Proxy proxy) {
      * @param freeFunc the specialized cleanup function to call
      */
     public static void setFreeFunc(MemorySegment address, String freeFunc) {
-        Cached cached = references.get(address);
-        references.put(address, new Cached(cached.owned, cached.references, freeFunc));
+        synchronized (references) {
+            Cached cached = references.get(address);
+            references.put(address, new Cached(cached.owned, cached.references, freeFunc));
+        }
     }
 
     /**
@@ -69,8 +73,10 @@ public static void setFreeFunc(MemorySegment address, String freeFunc) {
      * @param address the memory address
      */
     public static void takeOwnership(MemorySegment address) {
-        Cached cached = references.get(address);
-        references.put(address, new Cached(true, cached.references, cached.freeFunc));
+        synchronized (references) {
+            Cached cached = references.get(address);
+            references.put(address, new Cached(true, cached.references, cached.freeFunc));
+        }
     }
 
     /**
@@ -79,8 +85,10 @@ public static void takeOwnership(MemorySegment address) {
      * @param address the memory address
      */
     public static void yieldOwnership(MemorySegment address) {
-        Cached cached = references.get(address);
-        references.put(address, new Cached(false, cached.references, cached.freeFunc));
+        synchronized (references) {
+            Cached cached = references.get(address);
+            references.put(address, new Cached(false, cached.references, cached.freeFunc));
+        }
     }
 
     /**
@@ -92,39 +100,32 @@ public static void yieldOwnership(MemorySegment address) {
     private record Cached(boolean owned, int references, String freeFunc) {}
 
     /**
-     * This callback is run by the {@link Cleaner} when a struct or union instance
-     * has become unreachable, to free the native memory.
+     * This callback is run by the {@link Cleaner} when a struct or union instance has become unreachable, to free the
+     * native memory.
      */
-    private static class StructFinalizer implements Runnable {
-
-        private final MemorySegment address;
+    private record StructFinalizer(MemorySegment address) implements Runnable {
 
         /**
-         * Create a new StructFinalizer that will be run by the {@link Cleaner}
-         * @param address the native memory address
-         */
-        public StructFinalizer(MemorySegment address) {
-            this.address = address;
-        }
-
-        /**
-         * This method is run by the {@link Cleaner} when the last Proxy object for this
-         * memory address is garbage-collected.
+         * This method is run by the {@link Cleaner} when the last Proxy object for this memory address is
+         * garbage-collected.
          */
         public void run() {
-            Cached cached = references.get(address);
-
-            // When other references exist, decrease the refcount
-            if (cached.references > 1) {
-                references.put(address, new Cached(cached.owned, cached.references - 1, cached.freeFunc));
-                return;
+            Cached cached;
+            synchronized (references) {
+                cached = references.get(address);
+
+                // When other references exist, decrease the refcount
+                if (cached.references > 1) {
+                    references.put(address, new Cached(cached.owned, cached.references - 1, cached.freeFunc));
+                    return;
+                }
+
+                // When no other references exist, remove the address from the cache and free the memory
+                references.remove(address);
             }
 
-            // When no other references exist, remove the address from the cache and free the memory
-            references.remove(address);
-
             // if we don't have ownership, we must not run free()
-            if (! cached.owned) {
+            if (!cached.owned) {
                 return;
             }
 

diff --git a/glib/main/java/io/github/jwharm/javagi/types/TypeCache.java b/glib/main/java/io/github/jwharm/javagi/types/TypeCache.java
@@ -1,9 +1,9 @@
 package io.github.jwharm.javagi.types;
 
 import java.lang.foreign.MemorySegment;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 
 import org.gnome.glib.Type;
@@ -18,7 +18,7 @@
  */
 public class TypeCache {
 
-    private final static Map<Type, Function<MemorySegment, ? extends Proxy>> typeRegister = new HashMap<>();
+    private final static Map<Type, Function<MemorySegment, ? extends Proxy>> typeRegister = new ConcurrentHashMap<>();
 
     /**
      * Get the constructor from the type registry for the native object instance at the given