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

Fix memory leak on AOP Proxy class definition cache #27375

Merged
merged 1 commit into from Sep 13, 2021

Conversation

yokotaso
Copy link
Contributor

@yokotaso yokotaso commented Sep 9, 2021

Fix memory leak on AOP Proxy class definition cache.

How to reproduce

  1. Like @Validated annotation with Request scope Controller like bellow
// Annnotation definition
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Documented
@Controller
@Scope(WebApplicationContext.SCOPE_REQUEST)
@RequestMapping(method = { RequestMethod.GET, RequestMethod.HEAD })
@Transactional
@Validated
public @interface MyController {
}

@MyController
public class RootController {
    @ModelAttribute("requireJs")
    public boolean isRequireJs() {
        return true;
    }

    @RequestMapping("/search")
    public String search(
        Model model, 
        @RequestParam(value = "space", required = false) @Max(value=100) String space) {
        // ommit
    }
}
  1. Aop class definition cache won't work well, therefore memory leak

Why

this bug is intruced by:

  1. Because @Validated annotation added,
    AbstractAdvisingBeanPostProcessor#postProcessAfterInitialization add advice for @Validated

  2. Before Improve AdvisedSupport.getAdvisors() #26017,
    cache in org.springframework.cglib.core.AbstractClassGenerator works well.
    Because org.springframework.aop.framework.AdvisedSupport#getConfigurationOnlyCopy
    copy org.springframework.aop.framework.AdvisedSupport#advisorsArray for create copy instance.

But after #26017,
use same reference of org.springframework.aop.framework.AdvisedSupport#advisors for
create copy instance. and this advitors field update by AbstractAdvisingBeanPostProcessor#postProcessAfterInitialization

  1. When Create Aop class instance, try to reuse cache for class definition

Cache key is defined below code:

  • org.springframework.cglib.proxy.Enhancer#createClass
  • org.springframework.cglib.proxy.Enhancer#createHelper

org.springframework.cglib.proxy.Enhancer#filter is registered at
https://github.com/spring-projects/spring-framework/blob/main/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java#L201-L202

  1. cache miss for class definition, memory leak happen
    At AbstractAdvisingBeanPostProcessor#postProcessAfterInitialization, advice object
    updated and therefore cache key is pollute, cache miss and memory leak happen.
    memoryleak

So, When creating copy instance on org.springframework.aop.framework.AdvisedSupport#getConfigurationOnlyCopy,
It is good that create new instance of ArrayList and copy all advices.

This bug reported by @hikoma Thanks!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 9, 2021
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug labels Sep 9, 2021
@sbrannen
Copy link
Member

sbrannen commented Sep 9, 2021

Thanks for the PR and the detailed report.

We'll look into it.

@sbrannen sbrannen added this to the 5.3.10 milestone Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants