Skip to content

Fix leaking listener in Netty4HttpRequestBodyStream#112629

Merged
mhl-b merged 1 commit intoelastic:partial-rest-requestsfrom
mhl-b:leaking-close-listener
Sep 9, 2024
Merged

Fix leaking listener in Netty4HttpRequestBodyStream#112629
mhl-b merged 1 commit intoelastic:partial-rest-requestsfrom
mhl-b:leaking-close-listener

Conversation

@mhl-b
Copy link
Copy Markdown
Contributor

@mhl-b mhl-b commented Sep 7, 2024

Currently every instance of Netty4HttpRequestBodyStream will register close listener to channel, but never deregister. This PR removes listener when LastHttpContent is sent downstream to handler. Tested with debugger. Netty does not expose registered listeners.

Another minor change. Currently I setAutoRead(true) when stream receive LastHttpContent despite downstream state and current queueing. This PR setAutoRead(true) when LastHttpContent is sent to downstream, not on reception.

@mhl-b mhl-b added >non-issue :Distributed/Network Http and internode communication implementations Team:Distributed Meta label for distributed team. v8.16.0 labels Sep 7, 2024
@mhl-b mhl-b requested a review from Tim-Brooks September 7, 2024 00:58
@mhl-b mhl-b changed the title Fix leaking listener Fix leaking listener in Netty4HttpRequestBodyStream Sep 7, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Copy Markdown
Contributor

@Tim-Brooks Tim-Brooks left a comment

Choose a reason for hiding this comment

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

LGTM

@mhl-b mhl-b merged commit 2c234bf into elastic:partial-rest-requests Sep 9, 2024
Tim-Brooks pushed a commit that referenced this pull request Sep 17, 2024
Tim-Brooks pushed a commit that referenced this pull request Sep 18, 2024
Tim-Brooks pushed a commit that referenced this pull request Sep 18, 2024
Tim-Brooks pushed a commit to Tim-Brooks/elasticsearch that referenced this pull request Sep 19, 2024
Tim-Brooks added a commit that referenced this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Network Http and internode communication implementations >non-issue Team:Distributed Meta label for distributed team. v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants