Skip to content
This repository was archived by the owner on Feb 24, 2026. It is now read-only.

Fix bytebuf memory leak when a request is canceled#62

Merged
viacheslav-fomin-main merged 2 commits into
verygoodsecurity:vgs-editionfrom
viacheslav-fomin-main:defect/bb-leak-fix
Feb 28, 2019
Merged

Fix bytebuf memory leak when a request is canceled#62
viacheslav-fomin-main merged 2 commits into
verygoodsecurity:vgs-editionfrom
viacheslav-fomin-main:defect/bb-leak-fix

Conversation

@viacheslav-fomin-main
Copy link
Copy Markdown
Collaborator

@viacheslav-fomin-main viacheslav-fomin-main commented Feb 24, 2019

Fixes: https://app.clubhouse.io/vgs/story/12753/refactor-littleproxy

Another memory leak in lp. This time a message is not released when a request is canceled before the message was processed and sent further to SetupUpstream handler


if (ProxyUtils.isChunked(httpRequest)) {
process(ctx, httpRequest);
process(ctx, httpRequest, true);
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we need it so the message is not released when we use chunks (when we do not use custom executor)

ReferenceCountUtil.retain(httpRequest);

proxyServer.getMessageProcessingExecutor()
.execute(wrapTask(() -> {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we need to move wrapTask into try block so we release the message in case an exception happens there

}
final HttpResponse shortCircuitResponse = currentFilters.clientToProxyRequest(httpRequest);

if (!authenticationRequired) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

refactored authenticationRequired part, there was one extra if statement

@osklyarenko
Copy link
Copy Markdown

good job

@viacheslav-fomin-main viacheslav-fomin-main merged commit 54567fb into verygoodsecurity:vgs-edition Feb 28, 2019
@arushi315
Copy link
Copy Markdown

arushi315 commented Feb 14, 2020

Hi @viacheslav-fomin-main and @osklyarenko
We have been using Adam's littleproxy version and start seeing some memory leak issues.
While I was searching for some already reported bugs, I came across VGS's littleproxy version where I see two PRs have been raised for memory leak - this one and #48

Now we are evaluating VGS's littleproxy version and we are still seeing leaks. I have fixed one leak but there seems to be another place. I didn't understand the GlobalStateHandler role here.
If possible can you provide more details for it? an example on how you are using.
Thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants