diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/TypeCachingBytecodeGenerator.java b/src/main/java/org/mockito/internal/creation/bytebuddy/TypeCachingBytecodeGenerator.java index 30ed949bf5..8dcdcddd98 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/TypeCachingBytecodeGenerator.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/TypeCachingBytecodeGenerator.java @@ -15,8 +15,6 @@ class TypeCachingBytecodeGenerator extends ReferenceQueue implements BytecodeGenerator { - private static final Object BOOTSTRAP_LOCK = new Object(); - private final BytecodeGenerator bytecodeGenerator; private final TypeCache typeCache; @@ -35,17 +33,27 @@ public TypeCachingBytecodeGenerator(BytecodeGenerator bytecodeGenerator, boolean public Class mockClass(final MockFeatures params) { lock.readLock().lock(); try { - ClassLoader classLoader = params.mockedType.getClassLoader(); + Class mockedType = params.mockedType; + ClassLoader classLoader = mockedType.getClassLoader(); return (Class) typeCache.findOrInsert( classLoader, new MockitoMockKey( - params.mockedType, + mockedType, params.interfaces, params.serializableMode, params.stripAnnotations), () -> bytecodeGenerator.mockClass(params), - BOOTSTRAP_LOCK); + /* + * Only lock on the main mockType, instead of a global lock. + * But the mockType class lock, will still ensure that a MockitoMockKey will never be created twice. + * + * #3035: Excessive locking in TypeCachingBytecodeGenerator#BOOTSTRAP_LOCK + * + * Note: This lock is still a bit more than needed, if e.g. the same type is mocked with + * and without interfaces at the same time. But this can be neglected for simplicity. + */ + mockedType); } catch (IllegalArgumentException exception) { Throwable cause = exception.getCause(); if (cause instanceof RuntimeException) { diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/TypeCachingMockBytecodeGeneratorTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/TypeCachingMockBytecodeGeneratorTest.java index 026fba3bfb..7931de60ab 100644 --- a/src/test/java/org/mockito/internal/creation/bytebuddy/TypeCachingMockBytecodeGeneratorTest.java +++ b/src/test/java/org/mockito/internal/creation/bytebuddy/TypeCachingMockBytecodeGeneratorTest.java @@ -5,6 +5,9 @@ package org.mockito.internal.creation.bytebuddy; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.mockito.internal.creation.bytebuddy.MockFeatures.withMockFeatures; import static org.mockitoutil.ClassLoaders.inMemoryClassLoader; import static org.mockitoutil.SimpleClassGenerator.makeMarkerInterface; @@ -208,6 +211,36 @@ public void ensure_cache_returns_same_instance_defaultAnswer() throws Exception assertThat(cache).isEmpty(); } + @Test + public void validate_TypeCache_locks_on_MockClass_Issue3035() { + BytecodeGenerator bytecodeGeneratorMock = mock(); + TypeCachingBytecodeGenerator cachingMockBytecodeGenerator = + new TypeCachingBytecodeGenerator(bytecodeGeneratorMock, true); + + MockFeatures mockFeatures = + withMockFeatures( + TestInterface.class, + Collections.emptySet(), + SerializableMode.NONE, + false, + Answers.RETURNS_DEFAULTS); + + when(bytecodeGeneratorMock.mockClass(mockFeatures)) + .thenAnswer( + (invocation) -> { + assertThat(Thread.holdsLock(TestInterface.class)) + .overridingErrorMessage( + "Lock for TestInterface was not held by TypeCachingBytecodeGenerator") + .isTrue(); + return null; + }); + + cachingMockBytecodeGenerator.mockClass(mockFeatures); + verify(bytecodeGeneratorMock).mockClass(mockFeatures); + } + + interface TestInterface {} + static class HoldingAReference { final WeakReference> a;