Repository navigation
Fix @ConcurrencyLimit leak for non-singleton beans - #37652
Open
Ridzzz0Alam wants to merge 1 commit into
Open
Ridzzz0Alam wants to merge 1 commit into
Ridzzz0Alam wants to merge 1 commit into
Conversation
Prior to this commit, ConcurrencyLimitBeanPostProcessor applied a single shared interceptor that kept the concurrency state of every proxy in a strong IdentityHashMap keyed by proxy instance. Entries were never removed, so each prototype, request or session scoped bean with a `@ConcurrencyLimit` method was retained for the lifetime of the post-processor, leaking memory in proportion to the number of created instances. This commit introduces a protected getAdvisor(Advisor, Object, String) hook in AbstractAdvisingBeanPostProcessor, returning the shared advisor by default, and overrides it in ConcurrencyLimitBeanPostProcessor to apply a dedicated interceptor per proxy. The concurrency state is therefore held by the proxy itself and released with it, while throttling remains scoped per proxy instance as before. Signed-off-by: Ridzzz0Alam <ridwan0gts@gmail.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The issue
ConcurrencyLimitBeanPostProcessorretains every prototype, request or session scoped bean with a@ConcurrencyLimitmethod forever. After creating 1000 prototype instances and running GC all 1000 proxies were still reachable.Why it happened
Since gh-35788 one shared interceptor keeps each proxy's concurrency state in a strong
IdentityHashMapkeyed by proxy instance and entries are never removed. Strong references were the right fix for that race but the missing piece is a lifecycle for the entries.The fix
getAdvisor(Advisor, Object, String)hook inAbstractAdvisingBeanPostProcessorreturns the shared advisor by default.ConcurrencyLimitBeanPostProcessoroverrides it to apply a dedicated interceptor per proxy so the state lives on the proxy and is collected with it. The map is removed so throttling stays per proxy instance as before.Tests
ConcurrencyLimitTests#withPostProcessorForPrototypeBeans: fails before the fix, passes after../gradlew :spring-aop:check :spring-context:check :spring-tx:checkpass.Not covered