From 91c8e9c11754db7cdf4d715e38bcd0d6dc18faf6 Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Mon, 11 Feb 2019 14:30:10 -0800 Subject: [PATCH 1/9] Fixes #1614: Add API to clean up mocks. Due to the introduction of map from weak reference from mock instance to its invocation handler, Mockito became vunerable to memory leaks as there are multiple situations where Mockito could unintentionally hold strong references to mock instances in the map record. The strong references could be through spiedInstance for spies, and arguments used to faciliate method stubbing. Mockito could never know if the arguments passed in for method stubbing are also strongly referenced somewhere else or not, so Mockito needs to save a strong reference to these arguments to avoid premature GC. Therefore to solve cyclic strong references through arguments Mockito needs to explicitly know when mocks are out of their life, and clean up all internal strong references associated with them. This commit also fixes #1532 and fixes #1533. --- .../java/org/mockito/MockitoFramework.java | 22 +++++++++++ .../bytebuddy/InlineByteBuddyMockMaker.java | 13 ++++++- .../framework/DefaultMockitoFramework.java | 23 +++++++++++ .../org/mockito/plugins/InlineMockMaker.java | 35 +++++++++++++++++ .../InlineByteBuddyMockMakerTest.java | 25 ++++++++++++ .../DefaultMockitoFrameworkTest.java | 34 ++++++++++++++++ ...yclicMockMethodArgumentMemoryLeakTest.java | 39 +++++++++++++++++++ .../bugs/SelfSpyReferenceMemoryLeakTest.java | 35 +++++++++++++++++ 8 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/mockito/plugins/InlineMockMaker.java create mode 100644 subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java create mode 100644 subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java diff --git a/src/main/java/org/mockito/MockitoFramework.java b/src/main/java/org/mockito/MockitoFramework.java index 5ffe27215b..0ab81d78a2 100644 --- a/src/main/java/org/mockito/MockitoFramework.java +++ b/src/main/java/org/mockito/MockitoFramework.java @@ -92,4 +92,26 @@ public interface MockitoFramework { */ @Incubating InvocationFactory getInvocationFactory(); + + /** + * Clears up internal state of all existing mocks. This is useful when testing with {@link org.mockito.plugins.InlineMockMaker} + * which may hold references to mocks and leak memory. No interaction to any mock created previously is not allowed + * after calling this method. + * + * @since 2.24.8 + * @see #clearMock(Object) + */ + @Incubating + void clearAllMocks(); + + /** + * Clears up internal state of the mock. This is useful when testing with {@link org.mockito.plugins.InlineMockMaker} + * which may hold references to mocks and leak memory. No interaction to this mock is allowed after calling this method. + * + * @param mock the mock to clear up + * @since 2.24.8 + * @see #clearAllMocks() + */ + @Incubating + void clearMock(Object mock); } diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java b/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java index 8cb651a928..e99b915952 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMaker.java @@ -14,6 +14,7 @@ import org.mockito.internal.util.concurrent.WeakConcurrentMap; import org.mockito.invocation.MockHandler; import org.mockito.mock.MockCreationSettings; +import org.mockito.plugins.InlineMockMaker; import java.io.File; import java.io.FileOutputStream; @@ -89,7 +90,7 @@ * support this feature. */ @Incubating -public class InlineByteBuddyMockMaker implements ClassCreatingMockMaker { +public class InlineByteBuddyMockMaker implements ClassCreatingMockMaker, InlineMockMaker { private static final Instrumentation INSTRUMENTATION; @@ -271,6 +272,16 @@ public void resetMock(Object mock, MockHandler newHandler, MockCreationSettings } } + @Override + public void clearMock(Object mock) { + mocks.remove(mock); + } + + @Override + public void clearAllMocks() { + mocks.clear(); + } + @Override public TypeMockability isTypeMockable(final Class type) { return new TypeMockability() { diff --git a/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java b/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java index 69a733c077..585aa73ce6 100644 --- a/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java +++ b/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java @@ -10,6 +10,8 @@ import org.mockito.internal.util.Checks; import org.mockito.invocation.InvocationFactory; import org.mockito.listeners.MockitoListener; +import org.mockito.plugins.InlineMockMaker; +import org.mockito.plugins.MockMaker; import org.mockito.plugins.MockitoPlugins; import static org.mockito.internal.progress.ThreadSafeMockingProgress.mockingProgress; @@ -37,4 +39,25 @@ public MockitoPlugins getPlugins() { public InvocationFactory getInvocationFactory() { return new DefaultInvocationFactory(); } + + private InlineMockMaker getInlineMockMaker() { + MockMaker mockMaker = Plugins.getMockMaker(); + return (mockMaker instanceof InlineMockMaker) ? (InlineMockMaker) mockMaker : null; + } + + @Override + public void clearAllMocks() { + InlineMockMaker mockMaker = getInlineMockMaker(); + if (mockMaker != null) { + mockMaker.clearAllMocks(); + } + } + + @Override + public void clearMock(Object mock) { + InlineMockMaker mockMaker = getInlineMockMaker(); + if (mockMaker != null) { + mockMaker.clearMock(mock); + } + } } diff --git a/src/main/java/org/mockito/plugins/InlineMockMaker.java b/src/main/java/org/mockito/plugins/InlineMockMaker.java new file mode 100644 index 0000000000..e32918b81b --- /dev/null +++ b/src/main/java/org/mockito/plugins/InlineMockMaker.java @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2019 Mockito contributors + * This program is made available under the terms of the MIT License. + */ + +package org.mockito.plugins; + +import org.mockito.Incubating; + +/** + * Extension to {@link MockMaker} for mock makers that changes inline method implementations. + * @since 2.24.8 + */ +@Incubating +public interface InlineMockMaker extends MockMaker { + /** + * Clean up internal state for specified {@code mock}. You may assume there won't be any interaction to the specific + * mock after this is called. + * + * @param mock the mock instance whose internal state is to be cleaned. + * @since 2.24.8 + */ + @Incubating + void clearMock(Object mock); + + /** + * Cleans up internal state for all existing mocks. You may assume there won't be any interaction to mocks created + * previously after this is called. + * + * @since 2.24.8 + */ + @Incubating + void clearAllMocks(); + +} diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java index dfe3bb578a..e6d9f37d82 100644 --- a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java +++ b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java @@ -287,6 +287,31 @@ public void test_parameters_retention() throws Exception { .getOnly().getParameters().getOnly().getName()).isEqualTo("bar"); } + @Test + public void test_clear_mock_clears_handler() { + MockCreationSettings settings = settingsFor(GenericSubClass.class); + GenericSubClass proxy = mockMaker.createMock(settings, new MockHandlerImpl(settings)); + assertThat(mockMaker.getHandler(proxy)).isNotNull(); + mockMaker.clearMock(proxy); + assertThat(mockMaker.getHandler(proxy)).isNull(); + } + + @Test + public void test_clear_all_mock_clears_handler() { + MockCreationSettings settings = settingsFor(GenericSubClass.class); + GenericSubClass proxy1 = mockMaker.createMock(settings, new MockHandlerImpl(settings)); + assertThat(mockMaker.getHandler(proxy1)).isNotNull(); + + settings = settingsFor(GenericSubClass.class); + GenericSubClass proxy2 = mockMaker.createMock(settings, new MockHandlerImpl(settings)); + assertThat(mockMaker.getHandler(proxy1)).isNotNull(); + + mockMaker.clearAllMocks(); + + assertThat(mockMaker.getHandler(proxy1)).isNull(); + assertThat(mockMaker.getHandler(proxy2)).isNull(); + } + private static MockCreationSettings settingsFor(Class type, Class... extraInterfaces) { MockSettingsImpl mockSettings = new MockSettingsImpl(); mockSettings.setTypeToMock(type); diff --git a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java index 0937b69992..f5a36c9286 100644 --- a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java +++ b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java @@ -10,14 +10,18 @@ import org.mockito.MockSettings; import org.mockito.StateMaster; import org.mockito.exceptions.misusing.RedundantListenerException; +import org.mockito.internal.configuration.plugins.Plugins; import org.mockito.listeners.MockCreationListener; import org.mockito.listeners.MockitoListener; import org.mockito.mock.MockCreationSettings; +import org.mockito.plugins.InlineMockMaker; import org.mockitoutil.TestBase; import java.util.List; import java.util.Set; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.*; import static org.mockitoutil.ThrowableAssert.assertThat; @@ -112,5 +116,35 @@ public void run() { "For more information, see the javadoc for RedundantListenerException class."); } + @Test + public void clears_all_mocks() { + List list1 = mock(List.class); + assertTrue(mockingDetails(list1).isMock()); + List list2 = mock(List.class); + assertTrue(mockingDetails(list2).isMock()); + + framework.clearAllMocks(); + + if (Plugins.getMockMaker() instanceof InlineMockMaker) { + assertFalse(mockingDetails(list1).isMock()); + assertFalse(mockingDetails(list2).isMock()); + } + } + + @Test + public void clears_mock() { + List list1 = mock(List.class); + assertTrue(mockingDetails(list1).isMock()); + List list2 = mock(List.class); + assertTrue(mockingDetails(list2).isMock()); + + framework.clearMock(list1); + + if (Plugins.getMockMaker() instanceof InlineMockMaker) { + assertFalse(mockingDetails(list1).isMock()); + assertTrue(mockingDetails(list2).isMock()); + } + } + private static class MyListener implements MockitoListener {} } diff --git a/subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java b/subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java new file mode 100644 index 0000000000..b2b713f77f --- /dev/null +++ b/subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2019 Mockito contributors + * This program is made available under the terms of the MIT License. + */ + +package org.mockitoinline.bugs; + +import org.junit.Test; + +import static org.mockito.Mockito.framework; +import static org.mockito.Mockito.mock; + +public class CyclicMockMethodArgumentMemoryLeakTest { + private static final int ARRAY_LENGTH = 1 << 20; // 4 MB + + @Test + public void no_memory_leak_when_cyclically_calling_method_with_mocks() { + for (int i = 0; i < 100; ++i) { + final A a = mock(A.class); + a.largeArray = new int[ARRAY_LENGTH]; + final B b = mock(B.class); + + a.accept(b); + b.accept(a); + + framework().clearAllMocks(); + } + } + + private static class A { + private int[] largeArray; + + void accept(B b) {} + } + + private static class B { + void accept(A a) {} + } +} diff --git a/subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java b/subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java new file mode 100644 index 0000000000..270198e0b7 --- /dev/null +++ b/subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2019 Mockito contributors + * This program is made available under the terms of the MIT License. + */ + +package org.mockitoinline.bugs; + +import org.junit.Test; + +import static org.mockito.Mockito.framework; +import static org.mockito.Mockito.spy; + +public class SelfSpyReferenceMemoryLeakTest { + private static final int ARRAY_LENGTH = 1 << 20; // 4 MB + + @Test + public void no_memory_leak_when_spy_holds_reference_to_self() { + for (int i = 0; i < 100; ++i) { + final DeepRefSelfClass instance = spy(new DeepRefSelfClass()); + instance.refInstance(instance); + + framework().clearAllMocks(); + } + } + + private static class DeepRefSelfClass { + private final DeepRefSelfClass[] array = new DeepRefSelfClass[1]; + + private final int[] largeArray = new int[ARRAY_LENGTH]; + + private void refInstance(DeepRefSelfClass instance) { + array[0] = instance; + } + } +} From 808bae82f1401ce7dfbb23a9c861ac2462920437 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 2 Mar 2019 09:58:15 -0800 Subject: [PATCH 2/9] Tweaked the javadoc and method names --- .../java/org/mockito/MockitoFramework.java | 31 +++++++++++++------ .../framework/DefaultMockitoFramework.java | 4 +-- .../org/mockito/plugins/InlineMockMaker.java | 1 + .../DefaultMockitoFrameworkTest.java | 12 +++++-- ...yclicMockMethodArgumentMemoryLeakTest.java | 2 +- .../bugs/SelfSpyReferenceMemoryLeakTest.java | 2 +- 6 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/mockito/MockitoFramework.java b/src/main/java/org/mockito/MockitoFramework.java index 0ab81d78a2..719c88672a 100644 --- a/src/main/java/org/mockito/MockitoFramework.java +++ b/src/main/java/org/mockito/MockitoFramework.java @@ -94,24 +94,35 @@ public interface MockitoFramework { InvocationFactory getInvocationFactory(); /** - * Clears up internal state of all existing mocks. This is useful when testing with {@link org.mockito.plugins.InlineMockMaker} - * which may hold references to mocks and leak memory. No interaction to any mock created previously is not allowed - * after calling this method. + * Clears up internal state of all inline mocks. + * This method is only meaningful if inline mock maker is in use. + * Otherwise this method is a no-op and need not be used. + *

+ * This method is useful to tackle subtle memory leaks that are possible due to the nature of inline mocking + * (issue #1619). + * If you are facing those problems, call this method at the end of the test (or in "@After" method). + * See examples of using "clearInlineMocks" in Mockito test code. + *

+ * Mockito's "inline mocking" enables mocking final types, enums and final methods + * (read more in section 39 of {@link Mockito} javadoc). + * This method is only meaningful when {@link org.mockito.plugins.InlineMockMaker} is in use. + * If you're using a different {@link org.mockito.plugins.MockMaker} then this method is a no-op. * * @since 2.24.8 - * @see #clearMock(Object) + * @see #clearInlineMock(Object) */ @Incubating - void clearAllMocks(); + void clearInlineMocks(); /** - * Clears up internal state of the mock. This is useful when testing with {@link org.mockito.plugins.InlineMockMaker} - * which may hold references to mocks and leak memory. No interaction to this mock is allowed after calling this method. + * Clears up internal state of specific inline mock. + * This method is a single-mock variant of {@link #clearInlineMocks()}. + * Please read javadoc for {@link #clearInlineMocks()}. * - * @param mock the mock to clear up + * @param mock to clear up * @since 2.24.8 - * @see #clearAllMocks() + * @see #clearInlineMocks() */ @Incubating - void clearMock(Object mock); + void clearInlineMock(Object mock); } diff --git a/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java b/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java index 585aa73ce6..d92fc28734 100644 --- a/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java +++ b/src/main/java/org/mockito/internal/framework/DefaultMockitoFramework.java @@ -46,7 +46,7 @@ private InlineMockMaker getInlineMockMaker() { } @Override - public void clearAllMocks() { + public void clearInlineMocks() { InlineMockMaker mockMaker = getInlineMockMaker(); if (mockMaker != null) { mockMaker.clearAllMocks(); @@ -54,7 +54,7 @@ public void clearAllMocks() { } @Override - public void clearMock(Object mock) { + public void clearInlineMock(Object mock) { InlineMockMaker mockMaker = getInlineMockMaker(); if (mockMaker != null) { mockMaker.clearMock(mock); diff --git a/src/main/java/org/mockito/plugins/InlineMockMaker.java b/src/main/java/org/mockito/plugins/InlineMockMaker.java index e32918b81b..adbc5aeda8 100644 --- a/src/main/java/org/mockito/plugins/InlineMockMaker.java +++ b/src/main/java/org/mockito/plugins/InlineMockMaker.java @@ -13,6 +13,7 @@ */ @Incubating public interface InlineMockMaker extends MockMaker { + /** * Clean up internal state for specified {@code mock}. You may assume there won't be any interaction to the specific * mock after this is called. diff --git a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java index f5a36c9286..756eba6d37 100644 --- a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java +++ b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java @@ -22,7 +22,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockingDetails; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.withSettings; import static org.mockitoutil.ThrowableAssert.assertThat; public class DefaultMockitoFrameworkTest extends TestBase { @@ -123,7 +129,7 @@ public void clears_all_mocks() { List list2 = mock(List.class); assertTrue(mockingDetails(list2).isMock()); - framework.clearAllMocks(); + framework.clearInlineMocks(); if (Plugins.getMockMaker() instanceof InlineMockMaker) { assertFalse(mockingDetails(list1).isMock()); @@ -138,7 +144,7 @@ public void clears_mock() { List list2 = mock(List.class); assertTrue(mockingDetails(list2).isMock()); - framework.clearMock(list1); + framework.clearInlineMock(list1); if (Plugins.getMockMaker() instanceof InlineMockMaker) { assertFalse(mockingDetails(list1).isMock()); diff --git a/subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java b/subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java index b2b713f77f..439c888838 100644 --- a/subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java +++ b/subprojects/inline/src/test/java/org/mockitoinline/bugs/CyclicMockMethodArgumentMemoryLeakTest.java @@ -23,7 +23,7 @@ public void no_memory_leak_when_cyclically_calling_method_with_mocks() { a.accept(b); b.accept(a); - framework().clearAllMocks(); + framework().clearInlineMocks(); } } diff --git a/subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java b/subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java index 270198e0b7..f4bb55aea7 100644 --- a/subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java +++ b/subprojects/inline/src/test/java/org/mockitoinline/bugs/SelfSpyReferenceMemoryLeakTest.java @@ -19,7 +19,7 @@ public void no_memory_leak_when_spy_holds_reference_to_self() { final DeepRefSelfClass instance = spy(new DeepRefSelfClass()); instance.refInstance(instance); - framework().clearAllMocks(); + framework().clearInlineMocks(); } } From e44d76e7da56ec94ae2d26d111e0ad0fe6f6c246 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 2 Mar 2019 10:23:54 -0800 Subject: [PATCH 3/9] Replace conditional with JUnit assume --- .../DefaultMockitoFrameworkTest.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java index 756eba6d37..db47b2f15b 100644 --- a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java +++ b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; @@ -124,32 +125,40 @@ public void run() { @Test public void clears_all_mocks() { + //clearing mocks only works with inline mocking + assumeTrue(Plugins.getMockMaker() instanceof InlineMockMaker); + + //given List list1 = mock(List.class); assertTrue(mockingDetails(list1).isMock()); List list2 = mock(List.class); assertTrue(mockingDetails(list2).isMock()); + //when framework.clearInlineMocks(); - if (Plugins.getMockMaker() instanceof InlineMockMaker) { - assertFalse(mockingDetails(list1).isMock()); - assertFalse(mockingDetails(list2).isMock()); - } + //then + assertFalse(mockingDetails(list1).isMock()); + assertFalse(mockingDetails(list2).isMock()); } @Test public void clears_mock() { + //clearing mocks only works with inline mocking + assumeTrue(Plugins.getMockMaker() instanceof InlineMockMaker); + + //given List list1 = mock(List.class); assertTrue(mockingDetails(list1).isMock()); List list2 = mock(List.class); assertTrue(mockingDetails(list2).isMock()); + //when framework.clearInlineMock(list1); - if (Plugins.getMockMaker() instanceof InlineMockMaker) { - assertFalse(mockingDetails(list1).isMock()); - assertTrue(mockingDetails(list2).isMock()); - } + //then + assertFalse(mockingDetails(list1).isMock()); + assertTrue(mockingDetails(list2).isMock()); } private static class MyListener implements MockitoListener {} From a6027ac6acd6737032b8dbb839c7d98a492fa1b3 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 2 Mar 2019 13:35:18 -0800 Subject: [PATCH 4/9] Tweaked tests --- .../bytebuddy/InlineByteBuddyMockMakerTest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java index e6d9f37d82..eebdb962d7 100644 --- a/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java +++ b/src/test/java/org/mockito/internal/creation/bytebuddy/InlineByteBuddyMockMakerTest.java @@ -20,7 +20,11 @@ import org.mockito.mock.SerializableMode; import org.mockito.plugins.MockMaker; -import java.util.*; +import java.util.HashMap; +import java.util.List; +import java.util.Observable; +import java.util.Observer; +import java.util.Set; import java.util.regex.Pattern; import static net.bytebuddy.ClassFileVersion.JAVA_V8; @@ -292,7 +296,11 @@ public void test_clear_mock_clears_handler() { MockCreationSettings settings = settingsFor(GenericSubClass.class); GenericSubClass proxy = mockMaker.createMock(settings, new MockHandlerImpl(settings)); assertThat(mockMaker.getHandler(proxy)).isNotNull(); + + //when mockMaker.clearMock(proxy); + + //then assertThat(mockMaker.getHandler(proxy)).isNull(); } @@ -306,8 +314,10 @@ public void test_clear_all_mock_clears_handler() { GenericSubClass proxy2 = mockMaker.createMock(settings, new MockHandlerImpl(settings)); assertThat(mockMaker.getHandler(proxy1)).isNotNull(); + //when mockMaker.clearAllMocks(); + //then assertThat(mockMaker.getHandler(proxy1)).isNull(); assertThat(mockMaker.getHandler(proxy2)).isNull(); } From c3c0d0a9349be4feb45e9bb9b152f6473ad0d42c Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 2 Mar 2019 13:35:30 -0800 Subject: [PATCH 5/9] Tweaked javadoc --- .../java/org/mockito/MockitoFramework.java | 16 ++++++++++++++++ .../org/mockito/plugins/InlineMockMaker.java | 18 +++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/mockito/MockitoFramework.java b/src/main/java/org/mockito/MockitoFramework.java index 719c88672a..9ed3836e3b 100644 --- a/src/main/java/org/mockito/MockitoFramework.java +++ b/src/main/java/org/mockito/MockitoFramework.java @@ -102,12 +102,28 @@ public interface MockitoFramework { * (issue #1619). * If you are facing those problems, call this method at the end of the test (or in "@After" method). * See examples of using "clearInlineMocks" in Mockito test code. + * To find out why inline mock maker keeps track of the mock objects see {@link org.mockito.plugins.InlineMockMaker}. *

* Mockito's "inline mocking" enables mocking final types, enums and final methods * (read more in section 39 of {@link Mockito} javadoc). * This method is only meaningful when {@link org.mockito.plugins.InlineMockMaker} is in use. * If you're using a different {@link org.mockito.plugins.MockMaker} then this method is a no-op. * + * 


+     * public class ExampleTest {
+     *
+     *     @After
+     *     public void clearMocks() {
+     *         Mockito.framework().clearInlineMocks();
+     *     }
+     *
+     *     @Test
+     *     public void someTest() {
+     *         ...
+     *     }
+     * }
+     *
+ * * @since 2.24.8 * @see #clearInlineMock(Object) */ diff --git a/src/main/java/org/mockito/plugins/InlineMockMaker.java b/src/main/java/org/mockito/plugins/InlineMockMaker.java index adbc5aeda8..f964600f89 100644 --- a/src/main/java/org/mockito/plugins/InlineMockMaker.java +++ b/src/main/java/org/mockito/plugins/InlineMockMaker.java @@ -6,9 +6,25 @@ package org.mockito.plugins; import org.mockito.Incubating; +import org.mockito.MockitoFramework; /** - * Extension to {@link MockMaker} for mock makers that changes inline method implementations. + * Extension to {@link MockMaker} for mock makers that changes inline method implementations + * and need keep track of created mock objects. + *

+ * Mockito's default inline mock maker keeps track of created mock objects via weak reference map. + * This poses a risk of memory leaks in certain scenarios + * (issue #1619). + * There is no clean way to tackle those problems at the moment. + * Hence, {@code InlineMockMaker} interface exposes methods to explicitly clear mock references. + * Those methods are called by {@link MockitoFramework#clearInlineMocks()}. + * When the user encounters a leak, he can mitigate the problem with {@link MockitoFramework#clearInlineMocks()}. + *

+ * {@code InlineMockMaker} is for expert users and framework integrators, when custom inline mock maker is in use. + * If you have a custom {@link MockMaker} that keeps track of mock objects, + * please have your mock maker implement {@code InlineMockMaker} interface. + * This way, it can participate in {@link MockitoFramework#clearInlineMocks()} API. + * * @since 2.24.8 */ @Incubating From ebb9b8547422331b8b69d1702a4b62294060d288 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 2 Mar 2019 13:50:47 -0800 Subject: [PATCH 6/9] Javadoc and since tags --- src/main/java/org/mockito/Mockito.java | 12 +++++++++++- src/main/java/org/mockito/MockitoFramework.java | 6 ++++-- .../java/org/mockito/plugins/InlineMockMaker.java | 6 +++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/mockito/Mockito.java b/src/main/java/org/mockito/Mockito.java index e2a0ade374..0f5bb300f9 100644 --- a/src/main/java/org/mockito/Mockito.java +++ b/src/main/java/org/mockito/Mockito.java @@ -103,7 +103,8 @@ * 43. New API for integrations: MockitoSession is usable by testing frameworks (Since 2.15.+)
* 44. Deprecated org.mockito.plugins.InstantiatorProvider as it was leaking internal API. it was replaced by org.mockito.plugins.InstantiatorProvider2 (Since 2.15.4)
* 45. New JUnit Jupiter (JUnit5+) extension
- * 46. New Mockito.lenient() and MockSettings.lenient() methods (Since 2.20.0
+ * 46. New Mockito.lenient() and MockSettings.lenient() methods (Since 2.20.0)
+ * 47. New API for clearing mock state in inline mocking (Since 2.25.0)
* * *

0. Migrating to Mockito 2

@@ -1533,6 +1534,15 @@ * * For more information refer to {@link Mockito#lenient()}. * Let us know how do you find the new feature by opening a GitHub issue to discuss! + * + *

47. New API for clearing mock state in inline mocking (Since 2.25.0)

+ * + * In certain specific, rare scenarios (issue #1619) + * inline mocking causes memory leaks. + * There is no clean way to mitigate this problem completely. + * Hence, we introduced a new API to explicitly clear mock state (only make sense in inline mocking!). + * See example in {@link MockitoFramework#clearInlineMocks()}. + * If you have feedback or a better idea how to solve the problem please reach out. */ @SuppressWarnings("unchecked") public class Mockito extends ArgumentMatchers { diff --git a/src/main/java/org/mockito/MockitoFramework.java b/src/main/java/org/mockito/MockitoFramework.java index 9ed3836e3b..58cd4b67ea 100644 --- a/src/main/java/org/mockito/MockitoFramework.java +++ b/src/main/java/org/mockito/MockitoFramework.java @@ -124,7 +124,9 @@ public interface MockitoFramework { * } * * - * @since 2.24.8 + * If you have feedback or a better idea how to solve the problem please reach out. + * + * @since 2.25.0 * @see #clearInlineMock(Object) */ @Incubating @@ -136,7 +138,7 @@ public interface MockitoFramework { * Please read javadoc for {@link #clearInlineMocks()}. * * @param mock to clear up - * @since 2.24.8 + * @since 2.25.0 * @see #clearInlineMocks() */ @Incubating diff --git a/src/main/java/org/mockito/plugins/InlineMockMaker.java b/src/main/java/org/mockito/plugins/InlineMockMaker.java index f964600f89..8771aa777b 100644 --- a/src/main/java/org/mockito/plugins/InlineMockMaker.java +++ b/src/main/java/org/mockito/plugins/InlineMockMaker.java @@ -25,7 +25,7 @@ * please have your mock maker implement {@code InlineMockMaker} interface. * This way, it can participate in {@link MockitoFramework#clearInlineMocks()} API. * - * @since 2.24.8 + * @since 2.25.0 */ @Incubating public interface InlineMockMaker extends MockMaker { @@ -35,7 +35,7 @@ public interface InlineMockMaker extends MockMaker { * mock after this is called. * * @param mock the mock instance whose internal state is to be cleaned. - * @since 2.24.8 + * @since 2.25.0 */ @Incubating void clearMock(Object mock); @@ -44,7 +44,7 @@ public interface InlineMockMaker extends MockMaker { * Cleans up internal state for all existing mocks. You may assume there won't be any interaction to mocks created * previously after this is called. * - * @since 2.24.8 + * @since 2.25.0 */ @Incubating void clearAllMocks(); From cc38b6cce7cc1873a84df921872a065fd87836bd Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Sat, 2 Mar 2019 13:51:08 -0800 Subject: [PATCH 7/9] Next minor version --- version.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.properties b/version.properties index 54435d6413..21052c6954 100644 --- a/version.properties +++ b/version.properties @@ -1,5 +1,5 @@ #Currently building Mockito version -version=2.24.2 +version=2.25.0 #Previous version used to generate release notes delta previousVersion=2.24.1 From d61a8beb9a4bb8234e3685c9c55dff05de8d2673 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Tue, 5 Mar 2019 07:54:16 -0800 Subject: [PATCH 8/9] Added missing coverage --- .../internal/framework/DefaultMockitoFrameworkTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java index db47b2f15b..ae21488c93 100644 --- a/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java +++ b/src/test/java/org/mockito/internal/framework/DefaultMockitoFrameworkTest.java @@ -123,6 +123,15 @@ public void run() { "For more information, see the javadoc for RedundantListenerException class."); } + @Test + public void clearing_all_mocks_is_safe_regardless_of_mock_maker_type() { + List mock = mock(List.class); + + //expect + assertTrue(mockingDetails(mock).isMock()); + framework.clearInlineMocks(); + } + @Test public void clears_all_mocks() { //clearing mocks only works with inline mocking From 1641868a4651bf5c020f7afeb50fc728c3e38d27 Mon Sep 17 00:00:00 2001 From: Szczepan Faber Date: Tue, 5 Mar 2019 07:58:10 -0800 Subject: [PATCH 9/9] Javadoc --- src/main/java/org/mockito/Mockito.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/mockito/Mockito.java b/src/main/java/org/mockito/Mockito.java index 0f5bb300f9..2e10eca426 100644 --- a/src/main/java/org/mockito/Mockito.java +++ b/src/main/java/org/mockito/Mockito.java @@ -1541,7 +1541,7 @@ * inline mocking causes memory leaks. * There is no clean way to mitigate this problem completely. * Hence, we introduced a new API to explicitly clear mock state (only make sense in inline mocking!). - * See example in {@link MockitoFramework#clearInlineMocks()}. + * See example usage in {@link MockitoFramework#clearInlineMocks()}. * If you have feedback or a better idea how to solve the problem please reach out. */ @SuppressWarnings("unchecked")