Skip to content
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

Jump to bottom

memory leak #8765

Closed
zyb2013 opened this issue Jan 23, 2019 · 4 comments
Closed

memory leak #8765

zyb2013 opened this issue Jan 23, 2019 · 4 comments
Assignees
@trustin @normanmaurer @ejona86 @carl-mastrangelo

Comments

@zyb2013
Copy link

zyb2013 commented Jan 23, 2019 •
edited

image
image
In first picture,the code have no surround with try..catch.If here occurs RuntimeException,does it mean msg will not be released?
@normanmaurer
Copy link
Member

@zyb2013 you are right we are not consistent here... I wonder which implementation is more correct tho.

@ejona86 @carl-mastrangelo @trustin WDYT ?

@normanmaurer
Copy link
Member

And @Scottmitch

@Scottmitch
Copy link
Member

Since isNotValidPromise may throw, and doesn't take ownership of (or even know about) the msg we should be catching and releasing in writeAndFlush.

We could also consider moving the cleanup code into the write(Object, boolean, ChannelPromise) method which is called by both of these methods.

normanmaurer added a commit that referenced this issue Jan 23, 2019
@normanmaurer
Release message when validation of passed in ChannelPromise fails whe…
1a09a8d 
…n calling write(...) / writeAndFlush(...)

Motivation:

We need to release the message when we throw an IllegalArgumentException because of a validation failure of the promise to eliminate the risk of a memory leak.

Modifications:

- Consistently release the message before rethrow
- Add testcase.

Result:

Fixes #8765.
@normanmaurer normanmaurer mentioned this issue Jan 23, 2019
Merged
@carl-mastrangelo
Copy link
Member

Existing code that called writeAndFlush would wrap it correct?

normanmaurer added a commit that referenced this issue Jan 24, 2019
@normanmaurer
Release message when validation of passed in ChannelPromise fails whe…
3c2b863 
…n calling write(...) / writeAndFlush(...) (#8769)

Motivation:

We need to release the message when we throw an IllegalArgumentException because of a validation failure of the promise to eliminate the risk of a memory leak.

Modifications:

- Consistently release the message before rethrow
- Add testcase.

Result:

Fixes #8765.
normanmaurer added a commit that referenced this issue Jan 24, 2019
@normanmaurer
Release message when validation of passed in ChannelPromise fails whe…
c5c318f 
…n calling write(...) / writeAndFlush(...) (#8769)

Motivation:

We need to release the message when we throw an IllegalArgumentException because of a validation failure of the promise to eliminate the risk of a memory leak.

Modifications:

- Consistently release the message before rethrow
- Add testcase.

Result:

Fixes #8765.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@trustin trustin

@normanmaurer normanmaurer

@ejona86 ejona86

@carl-mastrangelo carl-mastrangelo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@trustin @normanmaurer @ejona86 @zyb2013 @Scottmitch @carl-mastrangelo