This is an automated email from the ASF dual-hosted git repository.
pandalee pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new df23b5d8 fix: fix WeakHashMap thread safety (#2366)
df23b5d8 is described below
commit df23b5d860458b08046e62e9447439f901620f1b
Author: Shawn Yang <[email protected]>
AuthorDate: Tue Jun 24 21:23:56 2025 +0800
fix: fix WeakHashMap thread safety (#2366)
## What does this PR do?
fix weak map thread safety
## Related issues
Closes #2367
## Does this PR introduce any user-facing change?
<!--
If any user-facing interface changes, please [open an
issue](https://github.com/apache/fory/issues/new/choose) describing the
need to do so and update the document if necessary.
-->
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
<!--
When the PR has an impact on performance (if you don't know whether the
PR will have an impact on performance, you can submit the PR first, and
if it will have impact on performance, the code reviewer will explain
it), be sure to attach a benchmark data here.
-->
---
.../org/apache/fory/codegen/CodeGenerator.java | 8 +-
.../org/apache/fory/collection/Collections.java | 12 +++
.../org/apache/fory/reflect/ReflectionUtils.java | 5 +-
.../main/java/org/apache/fory/type/Descriptor.java | 6 +-
.../java/org/apache/fory/util/LoaderBinding.java | 9 +-
.../org/apache/fory/util/function/Functions.java | 96 ++++++++++++++++------
6 files changed, 97 insertions(+), 39 deletions(-)
diff --git
a/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java
b/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java
index 68c84932..c1d22a24 100644
--- a/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java
+++ b/java/fory-core/src/main/java/org/apache/fory/codegen/CodeGenerator.java
@@ -480,8 +480,8 @@ public class CodeGenerator {
return sb;
}
- private static final WeakHashMap<Class<?>, Boolean>
sourcePublicLevelAccessible =
- new WeakHashMap<>();
+ private static final Map<Class<?>, Boolean> sourcePublicLevelAccessible =
+ Collections.newClassKeyCacheMap();
public static Class<?> getSourcePublicAccessibleParentClass(Class<?> clz) {
while (!sourcePublicAccessible(clz)) {
@@ -506,8 +506,8 @@ public class CodeGenerator {
return sourcePkgLevelAccessible(clz);
}
- private static final WeakHashMap<Class<?>, Boolean> sourcePkgLevelAccessible
=
- new WeakHashMap<>();
+ private static final Map<Class<?>, Boolean> sourcePkgLevelAccessible =
+ Collections.newClassKeyCacheMap();
/** Returns true if class is package level accessible from source. */
public static boolean sourcePkgLevelAccessible(Class<?> clz) {
diff --git
a/java/fory-core/src/main/java/org/apache/fory/collection/Collections.java
b/java/fory-core/src/main/java/org/apache/fory/collection/Collections.java
index 761ff0e5..6a8475d8 100644
--- a/java/fory-core/src/main/java/org/apache/fory/collection/Collections.java
+++ b/java/fory-core/src/main/java/org/apache/fory/collection/Collections.java
@@ -19,14 +19,18 @@
package org.apache.fory.collection;
+import com.google.common.collect.MapMaker;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
+import org.apache.fory.util.GraalvmSupport;
@SuppressWarnings({"rawtypes", "unchecked"})
public class Collections {
@@ -185,4 +189,12 @@ public class Collections {
}
return map;
}
+
+ public static <T> Map<Class<?>, T> newClassKeyCacheMap() {
+ if (GraalvmSupport.IN_GRAALVM_NATIVE_IMAGE) {
+ return new ConcurrentHashMap<>();
+ } else {
+ return new MapMaker().weakKeys().makeMap();
+ }
+ }
}
diff --git
a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java
b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java
index 69220535..e346822e 100644
--- a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java
+++ b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java
@@ -41,7 +41,6 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
-import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;
@@ -839,8 +838,8 @@ public class ReflectionUtils {
return Functions.isLambda(cls) || isJdkProxy(cls);
}
- private static final WeakHashMap<Class<?>, Boolean>
scalaSingletonObjectCache =
- new WeakHashMap<>();
+ private static final Map<Class<?>, Boolean> scalaSingletonObjectCache =
+ org.apache.fory.collection.Collections.newClassKeyCacheMap();
/** Returns true if a class is a scala `object` singleton. */
public static boolean isScalaSingletonObject(Class<?> cls) {
diff --git a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java
b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java
index a9cbe190..927ffda5 100644
--- a/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java
+++ b/java/fory-core/src/main/java/org/apache/fory/type/Descriptor.java
@@ -29,7 +29,6 @@ import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
@@ -39,7 +38,6 @@ import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.TreeSet;
-import java.util.WeakHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ForkJoinPool;
@@ -48,6 +46,7 @@ import org.apache.fory.annotation.Expose;
import org.apache.fory.annotation.ForyField;
import org.apache.fory.annotation.Ignore;
import org.apache.fory.annotation.Internal;
+import org.apache.fory.collection.Collections;
import org.apache.fory.collection.Tuple2;
import org.apache.fory.memory.Platform;
import org.apache.fory.reflect.TypeRef;
@@ -65,8 +64,7 @@ public class Descriptor {
private static Cache<
Class<?>, Tuple2<SortedMap<Member, Descriptor>, SortedMap<Member,
Descriptor>>>
descCache =
CacheBuilder.newBuilder().weakKeys().softValues().concurrencyLevel(64).build();
- private static final Map<Class<?>, AtomicBoolean> flags =
- Collections.synchronizedMap(new WeakHashMap<>());
+ private static final Map<Class<?>, AtomicBoolean> flags =
Collections.newClassKeyCacheMap();
@Internal
public static void clearDescriptorCache() {
diff --git
a/java/fory-core/src/main/java/org/apache/fory/util/LoaderBinding.java
b/java/fory-core/src/main/java/org/apache/fory/util/LoaderBinding.java
index 525f09e9..7f60e1f2 100644
--- a/java/fory-core/src/main/java/org/apache/fory/util/LoaderBinding.java
+++ b/java/fory-core/src/main/java/org/apache/fory/util/LoaderBinding.java
@@ -19,11 +19,13 @@
package org.apache.fory.util;
+import com.google.common.collect.MapMaker;
import java.lang.ref.SoftReference;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
-import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
import java.util.function.Function;
import org.apache.fory.Fory;
@@ -41,7 +43,10 @@ public final class LoaderBinding {
// which cause
// circular reference between ClassLoader and Fory.
private final HashMap<ClassLoader, Fory> foryMap = new HashMap<>();
- private final WeakHashMap<ClassLoader, SoftReference<Fory>> forySoftMap =
new WeakHashMap<>();
+ private final Map<ClassLoader, SoftReference<Fory>> forySoftMap =
+ GraalvmSupport.IN_GRAALVM_NATIVE_IMAGE
+ ? new ConcurrentHashMap<>()
+ : new MapMaker().weakKeys().makeMap();
private Consumer<Fory> bindingCallback = f -> {};
private ClassLoader loader;
private Fory fory;
diff --git
a/java/fory-core/src/main/java/org/apache/fory/util/function/Functions.java
b/java/fory-core/src/main/java/org/apache/fory/util/function/Functions.java
index c2100ded..0fd3f7b8 100644
--- a/java/fory-core/src/main/java/org/apache/fory/util/function/Functions.java
+++ b/java/fory-core/src/main/java/org/apache/fory/util/function/Functions.java
@@ -27,9 +27,9 @@ import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
+import org.apache.fory.collection.MultiKeyWeakMap;
import org.apache.fory.collection.Tuple2;
import org.apache.fory.reflect.ReflectionUtils;
import org.apache.fory.util.GraalvmSupport;
@@ -77,38 +77,82 @@ public class Functions {
}
}
- private static final Map<Tuple2<Method, Class<?>>, Object> map =
- GraalvmSupport.isGraalBuildtime() ? new ConcurrentHashMap<>() : new
WeakHashMap<>();
+ private static final Map<Tuple2<Method, Class<?>>, Object> graalvmCache =
+ GraalvmSupport.IN_GRAALVM_NATIVE_IMAGE ? new ConcurrentHashMap<>() :
null;
+ private static final MultiKeyWeakMap<Object> weakCache =
+ GraalvmSupport.IN_GRAALVM_NATIVE_IMAGE ? null : new MultiKeyWeakMap<>();
public static Object makeGetterFunction(Method method) {
- return map.computeIfAbsent(
- Tuple2.of(method, Object.class),
- k -> {
- MethodHandles.Lookup lookup =
_JDKAccess._trustedLookup(method.getDeclaringClass());
- try {
- // Why `lookup.findGetter` doesn't work?
- // MethodHandle handle =
lookup.findGetter(field.getDeclaringClass(), field.getName(),
- // field.getType());
- MethodHandle handle = lookup.unreflect(method);
- return _JDKAccess.makeGetterFunction(lookup, handle,
method.getReturnType());
- } catch (IllegalAccessException ex) {
- throw new RuntimeException(ex);
+ if (GraalvmSupport.IN_GRAALVM_NATIVE_IMAGE) {
+ return graalvmCache.computeIfAbsent(
+ Tuple2.of(method, Object.class),
+ k -> {
+ MethodHandles.Lookup lookup =
_JDKAccess._trustedLookup(method.getDeclaringClass());
+ try {
+ // Why `lookup.findGetter` doesn't work?
+ // MethodHandle handle =
lookup.findGetter(field.getDeclaringClass(), field.getName(),
+ // field.getType());
+ MethodHandle handle = lookup.unreflect(method);
+ return _JDKAccess.makeGetterFunction(lookup, handle,
method.getReturnType());
+ } catch (IllegalAccessException ex) {
+ throw new RuntimeException(ex);
+ }
+ });
+ } else {
+ Object[] keys = new Object[] {method, Object.class};
+ Object func = weakCache.get(keys);
+ if (func == null) {
+ synchronized (weakCache) {
+ func = weakCache.get(keys);
+ if (func == null) {
+ MethodHandles.Lookup lookup =
_JDKAccess._trustedLookup(method.getDeclaringClass());
+ try {
+ MethodHandle handle = lookup.unreflect(method);
+ func = _JDKAccess.makeGetterFunction(lookup, handle,
method.getReturnType());
+ weakCache.put(keys, func);
+ } catch (IllegalAccessException ex) {
+ throw new RuntimeException(ex);
+ }
}
- });
+ }
+ }
+ return func;
+ }
}
public static Object makeGetterFunction(Method method, Class<?> returnType) {
- return map.computeIfAbsent(
- Tuple2.of(method, returnType),
- k -> {
- MethodHandles.Lookup lookup =
_JDKAccess._trustedLookup(method.getDeclaringClass());
- try {
- MethodHandle handle = lookup.unreflect(method);
- return _JDKAccess.makeGetterFunction(lookup, handle, returnType);
- } catch (IllegalAccessException ex) {
- throw new RuntimeException(ex);
+ if (GraalvmSupport.IN_GRAALVM_NATIVE_IMAGE) {
+ return graalvmCache.computeIfAbsent(
+ Tuple2.of(method, returnType),
+ k -> {
+ MethodHandles.Lookup lookup =
_JDKAccess._trustedLookup(method.getDeclaringClass());
+ try {
+ MethodHandle handle = lookup.unreflect(method);
+ return _JDKAccess.makeGetterFunction(lookup, handle, returnType);
+ } catch (IllegalAccessException ex) {
+ throw new RuntimeException(ex);
+ }
+ });
+ } else {
+ Object[] keys = new Object[] {method, returnType};
+ Object func = weakCache.get(keys);
+ if (func == null) {
+ synchronized (weakCache) {
+ func = weakCache.get(keys);
+ if (func == null) {
+ MethodHandles.Lookup lookup =
_JDKAccess._trustedLookup(method.getDeclaringClass());
+ try {
+ MethodHandle handle = lookup.unreflect(method);
+ func = _JDKAccess.makeGetterFunction(lookup, handle, returnType);
+ weakCache.put(keys, func);
+ } catch (IllegalAccessException ex) {
+ throw new RuntimeException(ex);
+ }
}
- });
+ }
+ }
+ return func;
+ }
}
public static Tuple2<Class<?>, String> getterMethodInfo(Class<?> type) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
