Neil,
Regression test /test/java/util/zip/ZipFile/FinalizeInflater.java failed
with this fix.
The possible solution I can think of now is to do nothing but simply
return the inflater back
to the cache, but do an additional sanity check when "check out"
private Inflater getInflater() {
synchronized (inflaters) {
int size = inflaters.size();
while ((size = inflaters.size())> 0) {
Inflater inf = inflaters.remove(size - 1);
if (!inf.ended())
return inf;
}
return new Inflater(true);
}
}
http://cr.openjdk.java.net/~sherman/7031076/webrev/
What do you think? have a better idea?
Sherman
On 04-08-2011 5:36 AM, Neil Richards wrote:
On Thu, 2011-04-07 at 16:02 -0700, Xueming Shen wrote:
It appears it might not be necessary to do the finalize() in
ZipFileInflaterInputStream. The ZipFileInflaterInputStream itself does
not directly hold any native resource by itself that needs to be
released at the end of its life circle, if not closed explicitly. The
native resource/memory that need to be taken care of are held by its
fields "inf" and "zfin", which should be finalized by the
corresponding finalize() of their own classes (again, if not closed
explicitly), when their outer ZFIIS object is unreachable. The Inflater
class has its own finalize() implemented already to invoke its cleanup
method end(), so the only thing need to be addressed is to add the
finalize() into ZipFileInputStream class to call its close(), strictly
speaking this issue is not the regression caused by #6735255, we have
this "leak" before #6735255.
Also, would you like to consider to use WeakHeapMap<InputStream, Void>
instead of handling all the weak reference impl by yourself, the bonus
would be that the stalled entries might be cleaned up more frequently.
Hi Sherman,
Thanks for your continuing analysis of this change.
I concur with your assessment above, and agree that making the suggested
modifications to the changeset results in the code being simpler and
clearer.
Please find below an updated changeset incorporating these suggestions
(and rebased off jdk7-b136),
Let me know if you need anything else to progress this fix forward,
Thanks,
Neil
