Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Nested spies cause memory leaks #1532

Closed
arturdryomov opened this issue Nov 14, 2018 · 6 comments
Closed

Nested spies cause memory leaks #1532

arturdryomov opened this issue Nov 14, 2018 · 6 comments
Labels
bug

Comments

@arturdryomov
Copy link

Seems like nested spies can cause memory leaks since such objects are kept in memory without purging. Not sure if it can be resolved at all. Should it be avoided? Is there a mention about this in docs? Anyway, the code speaks better and fortunately I’ve been able to create a self-contained sample.

BTW I can provide a .hprof file if you are interested.

Versions

org.mockito:mockito-core:2.22.0
org.mockito:mockito-inline:2.22.0

java version "1.8.0_181"
Java(TM) SE Runtime Environment (build 1.8.0_181-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.181-b13, mixed mode)

Gradle

Heap is set to 64 MB.

tasks.withType<Test> {
    maxHeapSize = "64m"
    jvmArgs("-XX:+HeapDumpOnOutOfMemoryError")

    failFast = true
}
$ ./gradlew :module:cleanTestDebugUnitTest :module:testDebugUnitTest --tests "com.github.sample.NestedSpiesMemoryLeakSpec"

Code

package com.github.sample

import com.jakewharton.rxrelay2.BehaviorRelay
import io.reactivex.functions.Consumer
import org.jetbrains.spek.api.Spek
import org.jetbrains.spek.api.dsl.it
import org.junit.platform.runner.JUnitPlatform
import org.junit.runner.RunWith
import org.mockito.Mockito

@RunWith(JUnitPlatform::class)
class NestedSpiesMemoryLeakSpec : Spek({

    repeat(10_000) { iteration ->

        it("iteration [$iteration]") {
            Mockito.spy(Service())
        }

    }

}) {

    class Service {
        // Remove Mockito.spy and OOM disappears.
        val value = Mockito.spy(Consumer<Int> {
            // This closure keeps a reference to Service.
            streams.size
        })

        // See at as a mass to fill the RAM.
        val streams = (0..1_000).map { BehaviorRelay.create<Int>() }
    }

}
> Task :module:testDebugUnitTest
java.lang.OutOfMemoryError: GC overhead limit exceeded
Dumping heap to java_pid23350.hprof ...
Heap dump file created [99857779 bytes in 0.356 secs]

com.github.sample.NestedSpiesMemoryLeakSpec > it iteration [187] STANDARD_ERROR
    java.lang.OutOfMemoryError: GC overhead limit exceeded
    	at com.jakewharton.rxrelay2.BehaviorRelay.<init>(BehaviorRelay.java:99)
    	at com.jakewharton.rxrelay2.BehaviorRelay.create(BehaviorRelay.java:77)
    	at com.github.sample.NestedSpiesMemoryLeakSpec$Service.<init>(NestedSpiesMemoryLeakSpec.kt:32)
    	at com.github.sample.NestedSpiesMemoryLeakSpec$1$1$1.invoke(NestedSpiesMemoryLeakSpec.kt:17)
    	at com.github.sample.NestedSpiesMemoryLeakSpec$1$1$1.invoke(NestedSpiesMemoryLeakSpec.kt:12)
    	at org.jetbrains.spek.engine.Scope$Test.execute(Scope.kt:102)
    	at org.jetbrains.spek.engine.Scope$Test.execute(Scope.kt:80)
    	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:105)
    	at org.junit.platform.engine.support.hierarchical.NodeTestTask$$Lambda$82/547193480.execute(Unknown Source)
    	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:72)
    	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:95)
    	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:71)

com.github.sample.NestedSpiesMemoryLeakSpec > it iteration [187] FAILED
    java.lang.OutOfMemoryError

> Task :module:testDebugUnitTest FAILED

Eclipse Memory Analyzer

screen shot 2018-11-14 at 19 09 53

screen shot 2018-11-14 at 19 08 59

screen shot 2018-11-14 at 19 09 09

Seems like this happens:

  • Service is a spy.
  • Service contains a Consumer, it is a spy as well.
  • Consumer is a closure and has an implicit reference to Service.
  • Mockito keeps both spies and cannot remove them from memory since there is a cross-reference (I guess).
@arturdryomov
Copy link
Author

@mockitoguy, sorry for the ping, but the situation gets worse on our side. Is the issue with Mockito or we are just using it the wrong way?

@arturdryomov
Copy link
Author

CC @raphw

@raphw
Copy link
Member

raphw commented Jan 18, 2019

You are right, due to the reference of one mock to another, the weak map is loosing its effect.

Technically, we would need to make the spy reference weak to break the cross reference but I sm not sure how this can be achieved without risking to have the references collected prematurely.

@arturdryomov
Copy link
Author

@raphw, thanks for the explanation! If it is more or less intended — maybe let’s document it somewhere? Another option is throwing an exception or printing a warning in such cases. Since we weren’t aware of this and use Mockito quite a lot we got ourselves in a pretty bad OOM situation. Finding nested spies by hand is a tedeous task, not even sure I can automate it without library support. I’m afraid people will continue to do this not understanding the consequences.

Can I ask you to take a look at #1533 as well? I think it is pretty similar or even exactly the same in terms of the root issue.

@raphw
Copy link
Member

raphw commented Jan 19, 2019 •
edited

It is surely not expected behavior and I consider it a bug. However, I do not know how to solve this and I even doubt that it is solvable without a data structure that is called an https://en.wikipedia.org/wiki/Ephemeron and which Java does not (currently) support.

But we should definitely document this.

@ttanxu ttanxu mentioned this issue Feb 9, 2019
Closed
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 12, 2019
@ttanxu
Fixes mockito#1614: Clean up mocks when session finishes.
0f6aa89 
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 lifespan, and clean
up all internal strong references associated with them.

This commit also fixes mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 12, 2019
@ttanxu
Fixes mockito#1614: Clean up mocks when session finishes.
9f36f2c 
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 lifespan, and clean
up all internal strong references associated with them.

This commit also fixes mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 12, 2019
@ttanxu
Fixes mockito#1614: Clean up mocks when session finishes.
c131d98 
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 mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 12, 2019
@ttanxu
Fixes mockito#1614: Clean up mocks when session finishes.
59efe50 
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.

User can call MockitoSessionBuilder#trackAndCleanUpMocks() to let the
session track and clean up mocks. It also relies on implementation of
mock makers to cleanUpMock(Object). SubclassByteBuddyMockMaker is
a intentional no-op to avoid unnecessary behavior changes in stable
features.

This commit also fixes mockito#1532 and fixes mockito#1533.
@ttanxu ttanxu mentioned this issue Feb 12, 2019
Merged
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 12, 2019
@ttanxu
Fixes mockito#1614: Clean up mocks when session finishes.
5cf0331 
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.

User can call MockitoSessionBuilder#trackAndCleanUpMocks() to let the
session track and clean up mocks. It also relies on implementation of
mock makers to cleanUpMock(Object). SubclassByteBuddyMockMaker is
a intentional no-op to avoid unnecessary behavior changes in stable
features.

This commit also fixes mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 12, 2019
@ttanxu
Fixes mockito#1614: Clean up mocks when session finishes.
9343bec 
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.

User can call MockitoSessionBuilder#trackAndCleanUpMocks() to let the
session track and clean up mocks. It also relies on implementation of
mock makers to cleanUpMock(Object). SubclassByteBuddyMockMaker is
a intentional no-op to avoid unnecessary behavior changes in stable
features.

This commit also fixes mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 13, 2019
@ttanxu
Fixes mockito#1614: Clean up mocks when session finishes.
cbc1101 
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.

User can call MockitoSessionBuilder#trackAndCleanUpMocks() to let the
session track and clean up mocks.

This commit also fixes mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 28, 2019
@ttanxu
Fixes mockito#1614: Add API to clean up mocks.
c1ad3a0 
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 mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 28, 2019
@ttanxu
Fixes mockito#1614: Add API to clean up mocks.
f464f7a 
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 mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 28, 2019
@ttanxu
Fixes mockito#1614: Add API to clean up mocks.
a2fc2d6 
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 mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Feb 28, 2019
@ttanxu
Fixes mockito#1614: Add API to clean up mocks.
223c222 
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 mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Mar 1, 2019
@ttanxu
Fixes mockito#1614: Add API to clean up mocks.
85ea8eb 
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 mockito#1532 and fixes mockito#1533.
ttanxu added a commit to ttanxu/mockito that referenced this issue Mar 1, 2019
@ttanxu
Fixes mockito#1614: Add API to clean up mocks.
91c8e9c 
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 mockito#1532 and fixes mockito#1533.
@bric3 bric3 added the bug label Mar 6, 2019
@arturdryomov
Copy link
Author

Just checked Mockito 2.27.0. Using the new Mockito.framework().clearInlineMocks() call does the trick and helps to avoid OOM. Thanks!

@arturdryomov arturdryomov mentioned this issue Jun 11, 2019
Closed
@arthurscchan arthurscchan mentioned this issue Aug 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@arturdryomov @bric3 @raphw