search

Ninja-Squad / springmockk

SpringMockK: MockBean and SpyBean, but for MockK instead of Mockito
Apache License 2.0
487 stars 31 forks source link

Memory leak in com.ninjasquad.springmockk.MockkClear #97

Closed bakomchik closed 1 year ago

bakomchik commented 1 year ago

https://github.com/Ninja-Squad/springmockk/blob/master/src/main/kotlin/com/ninjasquad/springmockk/MockkClear.kt#L34 this maps holds all mocks for entire test phase. In our case we frequently spawns new application context which contains @Lazy beans, these beans keep references to BeanFactory. And despite the fact that application context is not needed anymore. MockkClear.clearModesByMock prevents objects to be garbage collected

jnizet commented 1 year ago

Hi @bakomchik

At first, I was under the impression that this wasn't really an issue in SpringMockK because Spring caches applications contexts used in tests in order to reuse them if another test uses the same context. See https://docs.spring.io/spring-framework/docs/6.0.x/reference/html/testing.html#testcontext-ctx-management-caching. Thus, retaining references to mocks wasn't a problem since Spring itself keeps a reference to them.

But reading this documentation, I realize that this cache is bounded to 32 instances by default. So this map would indeed be the only thing to keep references to mocks if a context is kicked out of the spring test context cache.

I have a fix in mind, consisting in replacing the identity map with a list of entries (mock + clear) where the mock would be held inside a weak reference.

Which version of Spring Boot and SpringMockK are you using? Ideally, I would only make that fix on the latest version, but the latest version targets Spring Boot 3.

bakomchik commented 1 year ago

Hi @jnizet,thank you for a quick response! And for the quite pretty library as well) In our cases, we have a lot of Test which dirties context. https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/test/annotation/DirtiesContext.html

currently we are using ` dependency("com.ninja-squad:springmockk:4.0.0")

` But we are working on migration to spring boot 3 . So if fix will be in latest version , it will be ok for us. I will think about fix also Thanks!

jnizet commented 1 year ago

Hi @bakomchik

I've implemented a fix for this issue, and I think it's right, but I would like to make sure it is. Could you please checkout the branch of the PR #98, build the library (using ./gradlew assemble) and test the change in your project?

bakomchik commented 1 year ago

Hi @jnizet . I tried. And it works! Thank you for your work and time!

jnizet commented 1 year ago

Great, thanks for the test.

I've published the release 4.0.1. It should appear in Maven central soon enough.