Release reactive connection that arrives after close#3372
Open
wushiyuanmaimob wants to merge 1 commit into
Open
Release reactive connection that arrives after close#3372wushiyuanmaimob wants to merge 1 commit into
wushiyuanmaimob wants to merge 1 commit into
Conversation
LettuceReactiveRedisConnection.AsyncConnect leaked a pooled connection when the connection acquisition was cancelled before the connection arrived from the LettuceConnectionProvider. The late-arriving connection was closed with closeAsync() but never released back to the provider, so a pooled provider kept counting it as active and eventually exhausted the pool. Release the late-arriving connection through connectionProvider.releaseAsync(it) instead, mirroring close(). For non-pooled providers this is equivalent to closing (the default releaseAsync delegates to closeAsync), and for pooled providers it returns the connection to the pool. Closes spring-projects#3371 Signed-off-by: sywu14 <sywu14@iflytek.com>
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.
Closes #3371
Problem
LettuceReactiveRedisConnection.AsyncConnectleaks a pooled connection when the connection acquisition is cancelled (request timeout, client disconnect, subscription cancellation) before the connection has arrived from theLettuceConnectionProvider.The late-arriving connection is closed via
it.closeAsync()but never released back to the provider, so a pooled provider keeps counting it as active (Lettuce'sBoundedAsyncPoolnever decrementsobjectCount/ removes it from theallqueue). Over time the pool is exhausted and throwsPoolException/Pool exhausted, even though the physical connections are already closed.Race scenario:
getConnection(); a connection is requested from the pool.this.connectionis stillnull.close()runs. Becausethis.connectionis stillnull,releaseAsync(...)is skipped and the state becomesCLOSED.doOnNextsees the closing state and callsit.closeAsync().releaseAsync(it)is never called, so the pool never reclaims the slot. Leak.Fix
Release the late-arriving connection through
connectionProvider.releaseAsync(it)instead ofit.closeAsync(), mirroring whatclose()already does for an already-arrived connection. For non-pooled providers this is equivalent to closing (the defaultreleaseAsyncdelegates tocloseAsync()); for pooled providers it correctly returns the connection to the pool.Tests
New unit test
shouldReleaseConnectionArrivingAfterClosereproduces the race using a deferred connection future: it subscribes, closes while the acquisition is in-flight, then completes the future and verifiesreleaseAsyncis invoked for the late-arriving connection. The test fails onmain(Wanted but not invoked: releaseAsync) and passes with this change.This issue was originally reported against Lettuce as redis/lettuce#3609, but the root cause is here in
AsyncConnect.