diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java index 030b835b08..f4ed2ae0b6 100644 --- a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java +++ b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java @@ -1,3 +1,19 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed 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 + * + * https://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.spockframework.mock.runtime; import net.bytebuddy.ByteBuddy; @@ -13,24 +29,56 @@ import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.implementation.bind.annotation.Morph; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.VisibleForTesting; import org.spockframework.mock.ISpockMockObject; import org.spockframework.mock.codegen.Target; import org.spockframework.util.Nullable; import java.lang.reflect.Method; import java.util.List; +import java.util.concurrent.Callable; import java.util.concurrent.ThreadLocalRandom; import static net.bytebuddy.matcher.ElementMatchers.any; import static net.bytebuddy.matcher.ElementMatchers.none; class ByteBuddyMockFactory { + /** + * The size of the {@link #cacheLocks}. + *

Caution: This must match the {@link #CACHE_LOCK_MASK}. + */ + private static final int CACHE_LOCK_SIZE = 16; + + /** + * The mask to use to mask out the {@link TypeCache.SimpleKey#hashCode()} to find the {@link #cacheLocks}. + *

Caution: this must match the bits of the {@link #CACHE_LOCK_SIZE}. + */ + private static final int CACHE_LOCK_MASK = 0x0F; + + /** + * This array contains {@link TypeCachingLock} instances, which are used as java monitor locks for + * {@link TypeCache#findOrInsert(ClassLoader, Object, Callable, Object)}. + * The {@code cacheLocks} spreads the lock to acquire over multiple locks instead of using a single lock + * {@code CACHE} for all {@code TypeCache.SimpleKeys}. + * + *

Note: We can't simply use the mockedType class lock as a lock, + * because the {@link TypeCache.SimpleKey}, will be the same for different {@code mockTypes + additionalInterfaces}. + * See the hashcode implementation of the {@code TypeCache.SimpleKey}, which has {@code Set} semantics. + */ + private static final TypeCachingLock[] cacheLocks; private static final TypeCache CACHE = new TypeCache.WithInlineExpunction<>(TypeCache.Sort.SOFT); private static final Class CODEGEN_TARGET_CLASS = Target.class; private static final String CODEGEN_PACKAGE = CODEGEN_TARGET_CLASS.getPackage().getName(); + static { + cacheLocks = new TypeCachingLock[CACHE_LOCK_SIZE]; + for (int i = 0; i < CACHE_LOCK_SIZE; i++) { + cacheLocks[i] = new TypeCachingLock(); + } + } + static Object createMock(final Class type, final List> additionalInterfaces, @Nullable List constructorArgs, @@ -38,8 +86,10 @@ static Object createMock(final Class type, final ClassLoader classLoader, boolean useObjenesis) { + TypeCache.SimpleKey key = new TypeCache.SimpleKey(type, additionalInterfaces); + Class enhancedType = CACHE.findOrInsert(classLoader, - new TypeCache.SimpleKey(type, additionalInterfaces), + key, () -> { String typeName = type.getName(); Class targetClass = type; @@ -68,14 +118,33 @@ static Object createMock(final Class type, .make() .load(classLoader, strategy) .getLoaded(); - }, CACHE); + }, getCacheLockForKey(key)); Object proxy = MockInstantiator.instantiate(type, enhancedType, constructorArgs, useObjenesis); ((ByteBuddyInterceptorAdapter.InterceptorAccess) proxy).$spock_set(interceptor); return proxy; } - // This methods and the ones it calls are inspired by similar code in Mockito's SubclassBytecodeGenerator + /** + * Returns a {@link TypeCachingLock}, which locks the {@link TypeCache#findOrInsert(ClassLoader, Object, Callable, Object)}. + * + * @param key the key to lock + * @return the {@link TypeCachingLock} to use to lock the {@link TypeCache} + */ + private static TypeCachingLock getCacheLockForKey(TypeCache.SimpleKey key) { + int hashCode = key.hashCode(); + // Try to spread some higher bits with XOR to lower bits, because we only use lower bits. + hashCode = hashCode ^ (hashCode >>> 16); + int index = hashCode & CACHE_LOCK_MASK; + return cacheLocks[index]; + } + + @VisibleForTesting + static void clearAllCaches() { + CACHE.clear(); + } + + // This method and the ones it calls are inspired by similar code in Mockito's SubclassBytecodeGenerator private static boolean shouldLoadIntoCodegenPackage(Class type) { return isComingFromJDK(type) || isComingFromSignedJar(type) || isComingFromSealedPackage(type); } @@ -112,4 +181,6 @@ private static ClassLoadingStrategy determineBestClassLoadingStrate return ClassLoadingStrategy.Default.WRAPPER; } + private static final class TypeCachingLock { + } } diff --git a/spock-specs/src/test/groovy/org/spockframework/mock/runtime/ByteBuddyMockFactoryConcurrentSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/mock/runtime/ByteBuddyMockFactoryConcurrentSpec.groovy new file mode 100644 index 0000000000..917e8e4c84 --- /dev/null +++ b/spock-specs/src/test/groovy/org/spockframework/mock/runtime/ByteBuddyMockFactoryConcurrentSpec.groovy @@ -0,0 +1,146 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed 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 + * + * https://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.spockframework.mock.runtime + +import groovy.transform.Canonical +import spock.lang.Isolated +import spock.lang.Shared +import spock.lang.Specification +import spock.lang.Timeout +import spock.lang.Unroll + +import java.util.concurrent.CompletableFuture +import java.util.concurrent.Phaser +import java.util.concurrent.TimeUnit +import java.util.function.Function + +// We need to run in isolation, because the ByteBuddyMockFactory is static and we interfere with the cache +// In the future we should change the ByteBuddyMockFactory to be an instance instead of static API. +@Isolated +class ByteBuddyMockFactoryConcurrentSpec extends Specification { + private static final String IfA = "IfA" + private static final String IfB = "IfB" + private static final String IfC = "IfC" + @Shared + final IProxyBasedMockInterceptor interceptor = Stub() + + // Just to be save to abort, normally the tests run in 2 secs. + @Timeout(40) + @Unroll("#test") + def "cacheLockingStressTest"(String test, MockSpec mockSpecA, MockSpec mockSpecB) { + setup: + int iterations = 5_000 + def tempClassLoader = new ByteBuddyTestClassLoader() + MockFeatures featA = toMockFeatures(mockSpecA, tempClassLoader) + MockFeatures featB = toMockFeatures(mockSpecB, tempClassLoader) + + Phaser phaser = new Phaser(4) + Function> runCode = { Runnable code -> + CompletableFuture.runAsync({ + phaser.arriveAndAwaitAdvance() + try { + for (int i = 0; i < iterations; i++) { + code.run() + } + } finally { + phaser.arrive() + } + }) + } + when: + def mockFeatAFuture = runCode.apply { + Class mockClass = mockClass(tempClassLoader, featA) + assertValidMockClass(featA, mockClass, tempClassLoader) + } + + def mockFeatBFuture = runCode.apply { + Class mockClass = mockClass(tempClassLoader, featB) + assertValidMockClass(featB, mockClass, tempClassLoader) + } + + def cacheFuture = runCode.apply(ByteBuddyMockFactory::clearAllCaches) + + phaser.arriveAndAwaitAdvance() + // Wait for test to end + int phase = phaser.arrive() + try { + phaser.awaitAdvanceInterruptibly(phase, 30, TimeUnit.SECONDS) + } finally { + // Collect exceptions from the futures, to make issues visible. + mockFeatAFuture.getNow(null) + mockFeatBFuture.getNow(null) + cacheFuture.getNow(null) + } + then: + noExceptionThrown() + + where: + test | mockSpecA | mockSpecB + "same hashcode different mockType" | mockSpec(IfA, IfB) | mockSpec(IfB, IfA) + "same hashcode same mockType" | mockSpec(IfA) | mockSpec(IfA) + "different hashcode different interfaces" | mockSpec(IfA, IfB) | mockSpec(IfB, IfC) + "unrelated classes" | mockSpec(IfA) | mockSpec(IfB) + } + + private Class mockClass(ClassLoader cl, MockFeatures feature) { + return ByteBuddyMockFactory.createMock( + feature.mockType, + feature.interfaces, + null, + interceptor, + cl, + false) + .getClass() + } + + private static MockSpec mockSpec(String mockedType, String... interfaces) { + return new MockSpec(mockedType, Arrays.asList(interfaces)) + } + + private void assertValidMockClass(MockFeatures mockFeature, Class mockClass, ClassLoader classLoader) { + assert mockClass.classLoader == classLoader + assert mockFeature.mockType.isAssignableFrom(mockClass) + for (Class anInterface : mockFeature.interfaces) { + assert anInterface.isAssignableFrom(mockClass) + } + } + + MockFeatures toMockFeatures(MockSpec mockFeaturesString, ByteBuddyTestClassLoader classLoader) { + def mockType = classLoader.defineInterface(mockFeaturesString.mockType) + def interfaces = mockFeaturesString.interfaces.collect { classLoader.defineInterface(it) } + return new MockFeatures(mockType, interfaces) + } + + /** + * Class holding the loaded classes to mock. + */ + @Canonical + private static class MockFeatures { + final Class mockType + final List> interfaces + } + + /** + * Class holding the class names to mock. + * Which will be converted into a {@link MockFeatures} during test. + */ + @Canonical + private static class MockSpec { + final String mockType + final List interfaces + } +} diff --git a/spock-specs/src/test/groovy/org/spockframework/mock/runtime/ByteBuddyTestClassLoader.java b/spock-specs/src/test/groovy/org/spockframework/mock/runtime/ByteBuddyTestClassLoader.java new file mode 100644 index 0000000000..371c303586 --- /dev/null +++ b/spock-specs/src/test/groovy/org/spockframework/mock/runtime/ByteBuddyTestClassLoader.java @@ -0,0 +1,55 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed 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 + * + * https://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.spockframework.mock.runtime; + +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; + +import java.util.HashMap; +import java.util.Map; + +final class ByteBuddyTestClassLoader extends ClassLoader { + private final Map> cache = new HashMap<>(); + + private final ClassLoadingStrategy loadingStrategy = (loader, types) -> { + Map> result = new HashMap<>(); + for (Map.Entry entry : types.entrySet()) { + TypeDescription description = entry.getKey(); + byte[] bytes = entry.getValue(); + Class clazz = defineClass(description.getName(), bytes, 0, bytes.length); + result.put(description, clazz); + } + return result; + }; + + /** + * Defines an empty interface with the passed {@code node}. + * + * @param name the name of the interface + * @return the loaded {@code Class} + */ + synchronized Class defineInterface(String name) { + //noinspection resource + return cache.computeIfAbsent(name, nameKey -> new ByteBuddy() + .makeInterface() + .name(nameKey) + .make() + .load(this, loadingStrategy) + .getLoaded()); + } +}