Skip to content

Commit 7de0a95

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 96f9247 commit 7de0a95

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
@@ -42,6 +42,7 @@ class JedisSubscription extends AbstractSubscription {
4242
*/
4343
@Override
4444
protected void doClose() {
45+
4546
if (!getChannels().isEmpty()) {
4647
jedisPubSub.unsubscribe();
4748
}

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
@@ -80,10 +80,6 @@ protected StatefulRedisPubSubConnection<byte[], byte[]> getNativeConnection() {
8080
@Override
8181
protected void doClose() {
8282

83-
if (!isAlive()) {
84-
return;
85-
}
86-
8783
List<CompletableFuture<?>> futures = new ArrayList<>();
8884

8985
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
@@ -103,8 +103,19 @@ protected AbstractSubscription(MessageListener listener, @Nullable byte[][] chan
103103
*/
104104
@Override
105105
public void close() {
106-
doClose();
107-
alive.set(false);
106+
107+
if (alive.compareAndSet(true, false)) {
108+
109+
doClose();
110+
111+
synchronized (channels) {
112+
channels.clear();
113+
}
114+
115+
synchronized (patterns) {
116+
patterns.clear();
117+
}
118+
}
108119
}
109120

110121
/**

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)