Skip to content

Release buffers in netty test #126744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 14, 2025
Merged

Release buffers in netty test #126744

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8e373f4
release buffers in netty test
mhl-b Apr 12, 2025
082c312
Merge branch 'main' into netty-test-fix
mhl-b Apr 14, 2025
d7beb99
Merge remote-tracking branch 'upstream/main' into netty-test-fix
mhl-b Apr 14, 2025
64c8aba
Merge branch 'netty-test-fix' of github.com:mhl-b/elasticsearch into …
mhl-b Apr 14, 2025
4fd12b2
feedback
mhl-b Apr 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions ...rt-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpHeaderValidatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ public void testMixedValidationResults() {
channel.writeInbound(last);
if (shouldPassValidation) {
assertEquals("should pass content for valid request", content, channel.readInbound());
content.release();
assertEquals(last, channel.readInbound());
last.release();
} else {
assertNull("should drop content for invalid request", channel.readInbound());
}
Expand All @@ -138,7 +140,10 @@ public void testIgnoreReadWhenValidating() {
assertNull("content should not pass yet, need explicit read", channel.readInbound());

channel.read();
assertTrue(channel.readInbound() instanceof LastHttpContent);
var lastContent = channel.readInbound();
assertTrue(lastContent instanceof LastHttpContent);
((LastHttpContent) lastContent).release();

Copy link
Contributor

@DaveCTurner DaveCTurner Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, in one line (with slightly nicer failure message too):

Suggested change
var lastContent = channel.readInbound();
assertTrue(lastContent instanceof LastHttpContent);
((LastHttpContent) lastContent).release();
asInstanceOf(LastHttpContent.class, channel.readInbound()).release();

}

public void testWithFlowControlAndAggregator() {
Expand All @@ -157,9 +162,10 @@ public void testWithFlowControlAndAggregator() {
validationRequest.listener.onResponse(null);
channel.runPendingTasks();

assertTrue(channel.readInbound() instanceof FullHttpRequest);
var fullReq = channel.readInbound();
assertTrue(fullReq instanceof FullHttpRequest);
((FullHttpRequest) fullReq).release();
Copy link
Contributor

@DaveCTurner DaveCTurner Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise

Suggested change
var fullReq = channel.readInbound();
assertTrue(fullReq instanceof FullHttpRequest);
((FullHttpRequest) fullReq).release();
asInstanceOf(FullHttpRequest.class, channel.readInbound()).release();

}

record ValidationRequest(HttpRequest request, Channel channel, ActionListener<Void> listener) {}

}