Suspected regression: fix for 6735255 causes delay in GC of ZipFile InputStreams, increase in heap demand

[Neil Richards]

On 18 April 2011 17:49, Xueming Shen <xueming.shen at oracle.com> wrote:
> If you are now explicitly synchronize on "streams" everywhere, I don't think
> we even need a copy
> at close().

I thought that the copy is needed so that the Map being iterated over
(in close()) is not able to be modified by the same thread via the
calls to InputStream.close() that it makes (which will try to call
streams.remove()), and hence avoid the risk of getting
ConcurrentModificationExceptions being thrown.

Are you sure that it is safe to avoid doing this copy ?

> To add "close() in ZFIIS.finalize() appears to be safe now (?, maybe I
> should think it again), but
> given we are now using WeakHashMap, those weak reference will get expunged
> every time that
> map gets accessed, I doubt that really help lots (have to wait for the GC
> kicks in anyway, if not
> closed explicitly)

As I recall, WeakHashMap only looks to expunge stale entries when one
of its methods is called.
Therefore, causing it to be accessed (indirectly) from the finalize()
methods (by calling close()) helps to ensure that stale entries (with
the retained inflater's associated system resources) are not retained
for longer than needed.

This is true even if the particular access is unlikely to retrieve a
"live" entry.

Hope this provides some clarification to the suggested changeset,
Neil

-- 
Unless stated above:
IBM email: neil_richards at uk.ibm.com
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU