Hi Roger,
I was wondering whether URLJarFile::finalize could be simply removed.
The change you are proposing looks strange to me: if this method
remains, then either the method should continue to call close() or
it should call super.finalize(), shouldn't it?
Since this is a sun.net class then maybe simply removing it would
be the safest thing...
best regards,
-- daniel
On 05/01/16 22:16, Roger Riggs wrote:
Hi Daniel,
webrev updated to revert changes to MeteredStream as too risky.
http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/index.html
On 1/5/2016 1:45 PM, Daniel Fuchs wrote:
Hi Roger,
Some early feedback:
I see that prior to your changes, MeteredStream.close() would
be called by finalize.
This will no longer be the case after
http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/index.html
In the case where finalize would call close(), there can be no
queuedForCleanup activity
because there can be no strong reference from the KeepAliveStreamCleaner
thread
or the queue that holds the pending cleanups.
It cannot be in the Cache of connections being kept alive or the thread
that keeps them alive.
It might be possible that the underlying connection is still open; but
there is no advantage
of trying to drain it. There is the possibility of resurrecting it by
virtue of deciding that
it should be queued for cleanup and starting a new thread to do the cleanup.
All in all, there is a risk of misunderstanding the dynamic behavior and
it would be safer
to leave this using finalize.
I see that MeteredStream has a subclass (KeepAliveStream) that
overrides close() - so KeepAliveStream probably requires a cleanup
as well - doesn't it?
Finally I'd suggest making the variable 'pi' final - since you're
passing that to the lambda you don't want to allow its value to
be modified afterwards (fortunately it's not - and I believe it
should be good to ensure it won't be).
ok, it would not go unnoticed, the compiler should complain if the
variable is no effectively final.
Thanks, Roger
I haven't looked at the other classes yet...
best regards,
-- daniel
On 05/01/16 19:24, Roger Riggs wrote:
The follow on work to adding the Cleaner is to replace uses of
finalization with uses of the Cleaner.
For the 'easy' cases in the java.base module, it is useful to introduce
a private Cleaner for the
java.base module. It is proposed to be held weakly, to allow it to
terminate on a lightly loaded
system.
Webrev for Review:
http://cr.openjdk.java.net/~rriggs/webrev-cleaning-factory-8146028/
The 2nd step is using the Cleaner.
- Empty finalize methods should (I think) be removed; but since they
are part of the public spec
the process needs two full releases; so the proposal is to deprecate
them first.
(The JEP 277 necessary changes will be updated when JEP 277
semantics are finalized)
- In a few cases, the code in the finalizer is redundant and if removed
would allow
an optimization of the handling of the finalizer and future removal
of the finalize method.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/index.html
Thanks for comments and suggestions, Roger
