From d88a50cec6618cfab1f5e08d5150f1c8273dfc95 Mon Sep 17 00:00:00 2001 From: Aliothmoon Date: Fri, 25 Oct 2024 15:51:55 +0800 Subject: [PATCH 1/4] fix(java): ThreadLocalFury and ThreadPoolFury prioritize using the user-specified ClassLoader --- .../java/org/apache/fury/ThreadLocalFury.java | 9 +++- .../fury/pool/FuryPooledObjectFactory.java | 8 +++ .../ThreadSafeFuryClassLoaderTest.java | 54 +++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java diff --git a/java/fury-core/src/main/java/org/apache/fury/ThreadLocalFury.java b/java/fury-core/src/main/java/org/apache/fury/ThreadLocalFury.java index d742650d94..78720b2e17 100644 --- a/java/fury-core/src/main/java/org/apache/fury/ThreadLocalFury.java +++ b/java/fury-core/src/main/java/org/apache/fury/ThreadLocalFury.java @@ -47,6 +47,8 @@ public class ThreadLocalFury extends AbstractThreadSafeFury { private Consumer factoryCallback; private final WeakHashMap allFury; + private ClassLoader classLoader; + public ThreadLocalFury(Function furyFactory) { factoryCallback = f -> {}; allFury = new WeakHashMap<>(); @@ -55,7 +57,11 @@ public ThreadLocalFury(Function furyFactory) { () -> { LoaderBinding binding = new LoaderBinding(furyFactory); binding.setBindingCallback(factoryCallback); - binding.setClassLoader(Thread.currentThread().getContextClassLoader()); + ClassLoader cl = + classLoader == null + ? Thread.currentThread().getContextClassLoader() + : classLoader; + binding.setClassLoader(cl); allFury.put(binding, null); return binding; }); @@ -254,6 +260,7 @@ public void setClassLoader(ClassLoader classLoader) { @Override public void setClassLoader(ClassLoader classLoader, StagingType stagingType) { + this.classLoader = classLoader; bindingThreadLocal.get().setClassLoader(classLoader, stagingType); } diff --git a/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java b/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java index 17e56a6348..93b1f9dc36 100644 --- a/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java +++ b/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java @@ -22,6 +22,7 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; import org.apache.fury.Fury; @@ -48,10 +49,16 @@ public class FuryPooledObjectFactory { */ final Cache classLoaderFuryPooledCache; + private final AtomicReference classLoaderRef = new AtomicReference<>(); + /** ThreadLocal: ClassLoader. */ private final ThreadLocal classLoaderLocal = ThreadLocal.withInitial( () -> { + ClassLoader cl = classLoaderRef.get(); + if (cl != null) { + return cl; + } ClassLoader loader = Thread.currentThread().getContextClassLoader(); if (loader == null) { loader = Fury.class.getClassLoader(); @@ -111,6 +118,7 @@ public void setClassLoader(ClassLoader classLoader, LoaderBinding.StagingType st // may be used to clear some classloader classLoader = Fury.class.getClassLoader(); } + classLoaderRef.set(classLoader); classLoaderLocal.set(classLoader); getOrAddCache(classLoader); } diff --git a/java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java b/java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java new file mode 100644 index 0000000000..24ea42c020 --- /dev/null +++ b/java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java @@ -0,0 +1,54 @@ +package org.apache.fury.classloader; + +import org.apache.fury.Fury; +import org.apache.fury.ThreadSafeFury; +import org.apache.fury.config.Language; +import org.testng.Assert; +import org.testng.annotations.Test; + +public class ThreadSafeFuryClassLoaderTest { + + static class MyClassLoader extends ClassLoader {} + + @Test + void testFuryThreadLocalUseProvidedClassLoader() throws InterruptedException { + final MyClassLoader myClassLoader = new MyClassLoader(); + final ThreadSafeFury fury = + Fury.builder() + .withClassLoader(myClassLoader) + .withLanguage(Language.JAVA) + .requireClassRegistration(false) + .buildThreadLocalFury(); + fury.setClassLoader(myClassLoader); + + Thread thread = + new Thread( + () -> { + final ClassLoader t = fury.getClassLoader(); + Assert.assertEquals(t, myClassLoader); + }); + thread.start(); + thread.join(); + } + + @Test + void testFuryPoolUseProvidedClassLoader() throws InterruptedException { + final MyClassLoader myClassLoader = new MyClassLoader(); + final ThreadSafeFury fury = + Fury.builder() + .withClassLoader(myClassLoader) + .withLanguage(Language.JAVA) + .requireClassRegistration(false) + .buildThreadSafeFuryPool(1, 1); + fury.setClassLoader(myClassLoader); + + Thread thread = + new Thread( + () -> { + final ClassLoader t = fury.getClassLoader(); + Assert.assertEquals(t, myClassLoader); + }); + thread.start(); + thread.join(); + } +} From ec133f21d334e2b33c7c0d92db573dcb9d46125f Mon Sep 17 00:00:00 2001 From: Aliothmoon Date: Fri, 25 Oct 2024 16:02:58 +0800 Subject: [PATCH 2/4] fix(java): Fix the license header in test files --- .../ThreadSafeFuryClassLoaderTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java b/java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java index 24ea42c020..f01ff02464 100644 --- a/java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java +++ b/java/fury-core/src/test/java/org/apache/fury/classloader/ThreadSafeFuryClassLoaderTest.java @@ -1,3 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.apache.fury.classloader; import org.apache.fury.Fury; From 48019cc2194b385d63ad1e8ed9761e136e1b684e Mon Sep 17 00:00:00 2001 From: Shawn Yang Date: Sat, 26 Oct 2024 00:58:27 +0800 Subject: [PATCH 3/4] Apply suggestions from code review --- .../org/apache/fury/pool/FuryPooledObjectFactory.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java b/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java index 93b1f9dc36..8d7bc1cf42 100644 --- a/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java +++ b/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java @@ -49,15 +49,14 @@ public class FuryPooledObjectFactory { */ final Cache classLoaderFuryPooledCache; - private final AtomicReference classLoaderRef = new AtomicReference<>(); + private volatile ClassLoader classLoader = null; /** ThreadLocal: ClassLoader. */ private final ThreadLocal classLoaderLocal = ThreadLocal.withInitial( () -> { - ClassLoader cl = classLoaderRef.get(); - if (cl != null) { - return cl; + if (classLoader != null) { + return classLoader; } ClassLoader loader = Thread.currentThread().getContextClassLoader(); if (loader == null) { @@ -118,7 +117,7 @@ public void setClassLoader(ClassLoader classLoader, LoaderBinding.StagingType st // may be used to clear some classloader classLoader = Fury.class.getClassLoader(); } - classLoaderRef.set(classLoader); + this.classLoader = classLoader; classLoaderLocal.set(classLoader); getOrAddCache(classLoader); } From 6253d1678dee57cb24386d796df0958da68fb5f3 Mon Sep 17 00:00:00 2001 From: Shawn Yang Date: Sat, 26 Oct 2024 01:04:07 +0800 Subject: [PATCH 4/4] Apply suggestions from code review --- .../main/java/org/apache/fury/pool/FuryPooledObjectFactory.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java b/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java index 8d7bc1cf42..9f8e2a2840 100644 --- a/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java +++ b/java/fury-core/src/main/java/org/apache/fury/pool/FuryPooledObjectFactory.java @@ -22,7 +22,6 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; import org.apache.fury.Fury;