On 9/29/17, 1:18 PM, Peter Levart wrote:
Hi Sherman,
putStream> streams = Collections.newSetFromMap(new WeakHashMap<>());
I also noticed that it is useless to test whether the inflater is
ended() when obtaining it from or releasing it into cache if the code
keeps the invariant that it never ends inflaters while they are still
in cache or associated with the open stream (the only place where
inflaters should be ended explicitly is in the Releaser). To make this
even more obvious, it might be good to move the obtaining/releasing
logic directly into the ZipFileInflaterInputStream constructor which
would be passed a reference to the inflatersCache instead of the
Inflater instance.
If my memory serves me correctly, this "ended()" check was for the
situation that the
ZipfileInflaterStream is getting finalized (close() is not called
explicitly/directly) and its
inflater is also getting finalized at the same time/pass, and the
inflater.end() gets called
(by its finalize()) first, so when the zfis tries to release the
inflater back to the cache, it has
been "ended" already. We had some nasty finalize() and object
resurrection timing
issue in earlier releases, that probably is part of the fix (since it's
a timing issue, we also
do the same "ended()" check in getInflater() when lease it out) So my
question is that do
we have the similar situation under the Cleaner? mean the zfis and its
inflater both are
phantom reachable and the inflater's cleaner gets called first?
I'm still reading your proposed change to remove the "inflater" from the
stream->inflater
mapping to see if there is any loophole, will reply that separately later.
-sherman
