From ee69e0dc93bfa700fff46266877dce47f10103d2 Mon Sep 17 00:00:00 2001 From: Jan-Willem Harmannij Date: Wed, 28 Jun 2023 21:47:22 +0200 Subject: [PATCH] Thread-safety improvements --- .../jwharm/javagi/interop/InstanceCache.java | 6 +- .../jwharm/javagi/interop/MemoryCleaner.java | 75 ++++++++++--------- .../github/jwharm/javagi/types/TypeCache.java | 4 +- 3 files changed, 43 insertions(+), 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 index 5f0a8a96..8c70179c 100644 --- 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 strongReferences = new HashMap<>(); - private final static Map> weakReferences = new HashMap<>(); + private final static Map strongReferences = new ConcurrentHashMap<>(); + private final static Map> 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 index 378dd25b..e6580e80 100644 --- 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 index 4436eb23..660ff35f 100644 --- 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> typeRegister = new HashMap<>(); + private final static Map> typeRegister = new ConcurrentHashMap<>(); /** * Get the constructor from the type registry for the native object instance at the given