Skip to content

Commit cd1260e

Browse files
committed
Unsubscribe only once on Subscription.close().
We now unsubscribe from Redis only once when closing a Subscription object to avoid duplicate traffic and unexpected Redis responses. Jedis does not have a mechanism to consume the additional unsubscribe response which leaves protocol frames on the InputStream leading to a corrupt state. Closes #2355
1 parent dce01ca commit cd1260e

File tree

4 files changed

+26
-6
lines changed

4 files changed

+26
-6
lines changed

src/main/java/org/springframework/data/redis/connection/jedis/JedisSubscription.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class JedisSubscription extends AbstractSubscription {
3838

3939
@Override
4040
protected void doClose() {
41+
4142
if (!getChannels().isEmpty()) {
4243
jedisPubSub.unsubscribe();
4344
}

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceSubscription.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,6 @@ protected StatefulRedisPubSubConnection<byte[], byte[]> getNativeConnection() {
7676
@Override
7777
protected void doClose() {
7878

79-
if (!isAlive()) {
80-
return;
81-
}
82-
8379
List<CompletableFuture<?>> futures = new ArrayList<>();
8480

8581
if (!getChannels().isEmpty()) {

src/main/java/org/springframework/data/redis/connection/util/AbstractSubscription.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,19 @@ protected AbstractSubscription(MessageListener listener, @Nullable byte[][] chan
9999

100100
@Override
101101
public void close() {
102-
doClose();
103-
alive.set(false);
102+
103+
if (alive.compareAndSet(true, false)) {
104+
105+
doClose();
106+
107+
synchronized (channels) {
108+
channels.clear();
109+
}
110+
111+
synchronized (patterns) {
112+
patterns.clear();
113+
}
114+
}
104115
}
105116

106117
/**

src/test/java/org/springframework/data/redis/connection/jedis/JedisSubscriptionUnitTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
* Unit test of {@link JedisSubscription}
3636
*
3737
* @author Jennifer Hickey
38+
* @author Mark Paluch
3839
*/
3940
@ExtendWith(MockitoExtension.class)
4041
class JedisSubscriptionUnitTests {
@@ -305,4 +306,15 @@ void testDoCloseSubscribedPatterns() {
305306
verify(jedisPubSub, times(1)).punsubscribe();
306307
}
307308

309+
@Test // GH-2355
310+
void closeTwiceShouldUnsubscribeOnce() {
311+
312+
subscription.subscribe(new byte[][] { "a".getBytes() });
313+
314+
subscription.close();
315+
subscription.close();
316+
317+
verify(jedisPubSub, times(1)).unsubscribe();
318+
}
319+
308320
}

0 commit comments

Comments
 (0)