spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.56k stars 40.55k forks source link

Restarter creates memory leak in tests #37373

Closed chris-melman closed 11 months ago

chris-melman commented 1 year ago

We run into memory problems on our buildstreet for automated tests. We are using an abstract base class (removed non spring functionality)

@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
@ActiveProfiles("test")
@DirtiesContext(classMode = ClassMode.BEFORE_EACH_TEST_METHOD) /
public abstract class AbstractEndToEndTest {

    @DynamicPropertySource
    static void registerDynamicProperties(DynamicPropertyRegistry registry) {
        registry.add("spring.datasource.url", () -> POSTGRESS_SQL_CONTAINER.getJdbcUrl());
        registry.add("spring.datasource.username", () -> POSTGRESS_SQL_CONTAINER.getUsername());
        registry.add("spring.datasource.password", () -> POSTGRESS_SQL_CONTAINER.getPassword());
    }

}

After some investigation we found that the org.springframework.boot.devtools.restart.Restarter is adding context to its rootContexts, however they never seems to be cleaned unnamed (2) unnamed (4)

https://github.com/spring-projects/spring-boot/blob/c4368bc934ab999bb7206fffd69007ef19adf483/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java#L122

Setting spring.test.context.cache.maxSize didn't help.

The current workaround is that we call the Restarter.clearInstance()after each test. However this is more a workaround. I would expect that the @SpringBootTest would clean this after the test has been run.

quaff commented 1 year ago

Just curious, why would you use devtools in tests?

chris-melman commented 1 year ago

Removing that dependency itself indeed solves the problem as wel. unnamed (5)

While indeed this is a dependency that is not required for test, we include in gradle to use it while developing. I could indeed add it conditionally that it is not included on CI. However, the memory leak is still there when we are running the test on the dev machines.

wilkinsona commented 1 year ago

@chris-melman if you declare DevTools in the developmentOnly configuration, it shouldn't appear on the test runtime classpath.

wilkinsona commented 1 year ago

If DevTools is on the test runtime classpath, the problem occurs even when the spring.devtools.restart.enabled system property is set to false.

wilkinsona commented 1 year ago

A couple of options:

  1. Listen to ContextClosedEvent and remove closed contexts from rootContexts
  2. Don't add contexts to rootContexts when the restarter is disabled. We may also want to use DevToolsEnablementDeducer to disable the restarter when running tests
chris-melman commented 1 year ago

developmentOnly is not a gradle default, however I see that is documented way to add the dependency to gradle

https://docs.spring.io/spring-boot/docs/2.7.15/reference/htmlsingle/#using.devtools

However this doesn't really work well with eclipse plugin of gradle.

philwebb commented 1 year ago

We're going to try option 2 and not add the contexts if the restarter is disabled.