Skip to content

Commit

Permalink
Fixes #1614: Add API to clean up mocks.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ttanxu committed Mar 1, 2019
1 parent f11705b commit 91c8e9c
Show file tree
Hide file tree
Showing 8 changed files with 225 additions and 1 deletion.
22 changes: 22 additions & 0 deletions src/main/java/org/mockito/MockitoFramework.java
Expand Up @@ -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);
}
Expand Up @@ -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;
Expand Down Expand Up @@ -89,7 +90,7 @@
* support this feature.
*/
@Incubating
public class InlineByteBuddyMockMaker implements ClassCreatingMockMaker {
public class InlineByteBuddyMockMaker implements ClassCreatingMockMaker, InlineMockMaker {

private static final Instrumentation INSTRUMENTATION;

Expand Down Expand Up @@ -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() {
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
35 changes: 35 additions & 0 deletions 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();

}
Expand Up @@ -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<GenericSubClass> settings = settingsFor(GenericSubClass.class);
GenericSubClass proxy = mockMaker.createMock(settings, new MockHandlerImpl<GenericSubClass>(settings));
assertThat(mockMaker.getHandler(proxy)).isNotNull();
mockMaker.clearMock(proxy);
assertThat(mockMaker.getHandler(proxy)).isNull();
}

@Test
public void test_clear_all_mock_clears_handler() {
MockCreationSettings<GenericSubClass> settings = settingsFor(GenericSubClass.class);
GenericSubClass proxy1 = mockMaker.createMock(settings, new MockHandlerImpl<GenericSubClass>(settings));
assertThat(mockMaker.getHandler(proxy1)).isNotNull();

settings = settingsFor(GenericSubClass.class);
GenericSubClass proxy2 = mockMaker.createMock(settings, new MockHandlerImpl<GenericSubClass>(settings));
assertThat(mockMaker.getHandler(proxy1)).isNotNull();

mockMaker.clearAllMocks();

assertThat(mockMaker.getHandler(proxy1)).isNull();
assertThat(mockMaker.getHandler(proxy2)).isNull();
}

private static <T> MockCreationSettings<T> settingsFor(Class<T> type, Class<?>... extraInterfaces) {
MockSettingsImpl<T> mockSettings = new MockSettingsImpl<T>();
mockSettings.setTypeToMock(type);
Expand Down
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {}
}
@@ -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) {}
}
}
@@ -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;
}
}
}

0 comments on commit 91c8e9c

Please sign in to comment.