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.

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

ZipInflaterInputStream native memory leak #13935

Closed
zhujibing opened this Issue Jul 28, 2018 · 5 comments

Comments

Projects
None yet
5 participants
@zhujibing
Copy link

zhujibing commented Jul 28, 2018

ZipInflaterInputStream constructor function will create Inflater object where use native memory,but Memory is not freed。

@zhujibing zhujibing changed the title ZipInflaterInputStream native ZipInflaterInputStream native memory leak Jul 28, 2018

@philwebb

This comment has been minimized.

Copy link
Member

philwebb commented Jul 28, 2018

Can you provide a few more details please. It's not clear to me why the Inflater won't be garbage collected in the same way as a regular object.

@zhujibing

This comment has been minimized.

Copy link
Author

zhujibing commented Aug 2, 2018

In ZipInflaterInputStream constructor function, we should set usesDefaultInflater = true; native memory can be freed when close is called. wo should not depend on gc. native memory is hard to release , because Inflater enter old generation.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Aug 6, 2018

We can't set usesDefaultInflater as it's a package-private field. We also aren't using the default Inflater; we're creating one with new Inflater(true).

wo should not depend on gc. native memory is hard to release , because Inflater enter old generation.

The GC generation of the object should not make any difference. AFAIK, its memory (and any associated native memory) will be freed when it is eligible for garbage collection.

Rather than describing a theoretical problem, perhaps you can provide a small sample that shows a failure due to memory being leaked?

@izeye

This comment has been minimized.

Copy link
Contributor

izeye commented Aug 6, 2018

I guess @zhujibing tried to say a general rule that releasing resources explicitly whenever possible is better than resorting to GC, so I just created #14001 to resolve the concern.

@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Aug 7, 2018

Closing in favour of #14001.

@wilkinsona wilkinsona closed this Aug 7, 2018

snicoll added a commit that referenced this issue Aug 8, 2018

Merge pull request #14001 from izeye:gh-13935
* pr/14001:
  Invoke Inflater.end() in ZipInflaterInputStream.close()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment