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.

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

Restarter creates memory leak in tests #37373

Closed
chris-melman opened this issue Sep 13, 2023 · 7 comments
Closed

Restarter creates memory leak in tests #37373

chris-melman opened this issue Sep 13, 2023 · 7 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@chris-melman
Copy link

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)

private final List<ConfigurableApplicationContext> rootContexts = new CopyOnWriteArrayList<>();

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 13, 2023
@quaff
Copy link
Contributor

quaff commented Sep 14, 2023

Just curious, why would you use devtools in tests?

@chris-melman
Copy link
Author

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
Copy link
Member

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

@wilkinsona wilkinsona self-assigned this Sep 14, 2023
@wilkinsona
Copy link
Member

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 wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 14, 2023
@wilkinsona wilkinsona added this to the 2.7.x milestone Sep 14, 2023
@wilkinsona
Copy link
Member

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

@wilkinsona wilkinsona removed their assignment Sep 14, 2023
@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Sep 14, 2023
@chris-melman
Copy link
Author

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants