Skip to content

Commit 55361fa

Browse files
committed
Release RSocket setup payload when it is not handled
This commit counterbalances the retain in the MessagingRSocket handleConnectionSetupPayload method with a conditional release on SETUP frame type in handleNoMatch, preventing Netty buffer leaks. Closes gh-32424
1 parent 546ca9b commit 55361fa

File tree

2 files changed

+45
-2
lines changed

2 files changed

+45
-2
lines changed

spring-messaging/src/main/java/org/springframework/messaging/rsocket/annotation/support/RSocketMessageHandler.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -37,6 +37,7 @@
3737
import org.springframework.core.annotation.AnnotatedElementUtils;
3838
import org.springframework.core.codec.Decoder;
3939
import org.springframework.core.codec.Encoder;
40+
import org.springframework.core.io.buffer.PooledDataBuffer;
4041
import org.springframework.lang.Nullable;
4142
import org.springframework.messaging.Message;
4243
import org.springframework.messaging.MessageDeliveryException;
@@ -381,6 +382,9 @@ else if (parameter.nested().getNestedParameterType().equals(Void.class)) {
381382
protected void handleNoMatch(@Nullable RouteMatcher.Route destination, Message<?> message) {
382383
FrameType frameType = RSocketFrameTypeMessageCondition.getFrameType(message);
383384
if (frameType == FrameType.SETUP || frameType == FrameType.METADATA_PUSH) {
385+
if (frameType == FrameType.SETUP && message.getPayload() instanceof PooledDataBuffer pooledDataBuffer) {
386+
pooledDataBuffer.release();
387+
}
384388
return; // optional handling
385389
}
386390
if (frameType == FrameType.REQUEST_FNF) {

spring-messaging/src/test/java/org/springframework/messaging/rsocket/annotation/support/RSocketMessageHandlerTests.java

+40-1
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@
1616

1717
package org.springframework.messaging.rsocket.annotation.support;
1818

19+
import java.nio.charset.StandardCharsets;
1920
import java.util.Arrays;
2021
import java.util.Collections;
2122
import java.util.Map;
2223

24+
import io.netty.buffer.UnpooledByteBufAllocator;
2325
import io.rsocket.frame.FrameType;
2426
import org.junit.jupiter.api.Test;
2527
import reactor.core.publisher.Flux;
@@ -33,6 +35,8 @@
3335
import org.springframework.core.codec.ByteBufferEncoder;
3436
import org.springframework.core.codec.CharSequenceEncoder;
3537
import org.springframework.core.codec.StringDecoder;
38+
import org.springframework.core.io.buffer.NettyDataBuffer;
39+
import org.springframework.core.io.buffer.NettyDataBufferFactory;
3640
import org.springframework.messaging.Message;
3741
import org.springframework.messaging.handler.CompositeMessageCondition;
3842
import org.springframework.messaging.handler.DestinationPatternsMessageCondition;
@@ -250,6 +254,10 @@ void handleNoMatch() {
250254
}
251255

252256
private static void testHandleNoMatch(FrameType frameType) {
257+
testHandleNoMatch(frameType, "");
258+
}
259+
260+
private static void testHandleNoMatch(FrameType frameType, Object payload) {
253261
RSocketMessageHandler handler = new RSocketMessageHandler();
254262
handler.setDecoders(Collections.singletonList(StringDecoder.allMimeTypes()));
255263
handler.setEncoders(Collections.singletonList(CharSequenceEncoder.allMimeTypes()));
@@ -260,11 +268,42 @@ private static void testHandleNoMatch(FrameType frameType) {
260268

261269
MessageHeaderAccessor headers = new MessageHeaderAccessor();
262270
headers.setHeader(RSocketFrameTypeMessageCondition.FRAME_TYPE_HEADER, frameType);
263-
Message<Object> message = MessageBuilder.createMessage("", headers.getMessageHeaders());
271+
Message<Object> message = MessageBuilder.createMessage(payload, headers.getMessageHeaders());
264272

265273
handler.handleNoMatch(route, message);
266274
}
267275

276+
@Test
277+
void handleNoMatchWithNettyBufferPayload() {
278+
279+
testHandleNoMatchBuffer(FrameType.SETUP, true);
280+
testHandleNoMatchBuffer(FrameType.METADATA_PUSH, false);
281+
testHandleNoMatchBuffer(FrameType.REQUEST_FNF, false);
282+
283+
assertThatThrownBy(() -> testHandleNoMatchBuffer(FrameType.REQUEST_RESPONSE, false))
284+
.hasMessage("No handler for destination 'path'");
285+
}
286+
287+
private static void testHandleNoMatchBuffer(FrameType frameType, boolean expectReleased) {
288+
NettyDataBufferFactory factory = new NettyDataBufferFactory(UnpooledByteBufAllocator.DEFAULT);
289+
NettyDataBuffer buf = factory.allocateBuffer(5);
290+
buf.write("hello", StandardCharsets.UTF_8);
291+
292+
assertThat(buf.getNativeBuffer().refCnt()).as(frameType + " refCnt").isOne();
293+
294+
try {
295+
testHandleNoMatch(frameType, buf);
296+
}
297+
finally {
298+
if (expectReleased) {
299+
assertThat(buf.getNativeBuffer().refCnt()).as(frameType + " is released").isZero();
300+
}
301+
else {
302+
assertThat(buf.getNativeBuffer().refCnt()).as("is not released").isOne();
303+
}
304+
}
305+
}
306+
268307

269308
private static class SimpleController {
270309

0 commit comments

Comments
 (0)