Xueming Shen said the following on 04/02/11 05:07:
On 04/01/2011 09:42 AM, Neil Richards wrote:
On Wed, 2011-03-30 at 13:31 -0700, Xueming Shen wrote:
Isn't it true that when the finalize()->close() gets invoked, there
should be no strong reference anywhere else that you can use to invoke
close() in other thread?
It's true that once finalize() has been called, there can't be another
explicit call to close().
However, if close() is explicitly called first, it will be called again
when finalize() calls it, so one still wants the update to 'isClosed' to
be seen by the finalizer thread (in this case).
I'm not a GC guy, so I might be missing something here, but if close()
is being explicitly
invoked by some thread, means someone has a strong reference to it, I
don't think the
finalize() can kick in until that close() returns and the strong
reference used to make that
explicit invocation is cleared. The InputStream is eligible for
finalization only after it is
"weakly" reachable, means no more "stronger" reachable exists, right?
Actually no. One of the more obscure corner cases with finalization is
that you can actually finalize an object that is still being used. The
JLS actually spells this out - see section 12.6.1 and in particular the
Discussion within that section.
David
-sherman
ZipFileInputStream class has to sync on ZipFile.this, the read/close()
methods are accessing the underlying/shared bits of the ZipFile. For
ZipFileInflaterInputStream.close() case, even we want to make sure the
isClose is synced, I would prefer to see a "local" lock, maybe simply
make close() synchronized is better?
After assessing it again, I tend to agree - synchronizing on the ZFIIS
(instead of the ZipFile) now looks to me to be safe because it does not
do anything which synchronizes on the ZipFile object.
(Note that if it _did_ cause such synchronization, it would risk
deadlock with the code in ZipFile's close() method, which is
synchronized on itself when it calls close() on each remaining ZFIIS.
It's the difference in the order of acquiring the monitors that would
cause the potential for deadlock).
As it is, both close() methods synchronize on the 'inflaters' object
within their respective synchronized blocks, but that does not risk
deadlock.
Please find below an updated changeset below, containing the suggested
change to synchronization.
Thanks once again for your suggestions,
Neil
