Re: RFR 8080225 FileInput/OutputStream/FileChannel cleanup should be improved
Hi Rogger, On 12/04/2017 02:17 PM, Peter Levart wrote:
Hi Rogger,
Interesting approach. Conditional finalization. You use finalization to support cases where user overrides finalize() and/or close() and Cleaner when he doesn't.
I wonder if it is the right thing to use AltFinalizer when user overrides finalize() method. In that case the method is probably not empty and calls super.finalize() (if it is empty or doesn't call super, user probably doesn't want the finalization to close the stream) and so normal finalization applies. If you register AltFinalizer for such case, close() will be called twice.
Ah, scrap that. I forgot that XXXStream.finalize() is now empty, so user overriding it and calling super does not in fact close the stream. You have to register AltFinalizer in that case. But now I wonder if the logic should still be 3-state and do the following: - if user overrides finalize() - use AltFinalizer to call both: first finalize() and then close(); else - if user overrides close() - use AltFinalizer to call close(); else - use Cleaner What do you think? Regards, Peter
This is a nice exercise in compatible migration from finalize() to Cleaner. The compatibility would be even better with a little tweak to the VM. Currently VM ignores empty finalize() method(s) and does not register the instance for finalization if it has empty finalize() (just return bytecode). There could be a special method-level runtime annotation, say @IgnoreFinalize that would be used to annotate no-empty finalize() methods and would have the same effect as empty method on the VM behavior. XXXStream could then just annotate the finalize() and leave it as is so that potential overriders of finalize() could call super. Current approach (with AltFinalizer) is an approximation since it can only call finalize() and close() in succession. If overridden finalize() callse super.finalize() in the middle of the method, it may expect the stream to be already closed for the rest of the method: @Override protected finalize() { ... ... pre-close logic ... super.finalize(); ... ... post-close logic (may expect stream to be already closed) ... } ...bust since super.close() is empty, the stream is not closed yet and will be closed by AltFinalizer after this.finalize() returns. I don't know what impact does such order have on the compatibility. Probably not big. Regards, Peter On 12/04/2017 02:25 PM, Peter Levart wrote:
Hi Rogger,
On 12/04/2017 02:17 PM, Peter Levart wrote:
Hi Rogger,
Interesting approach. Conditional finalization. You use finalization to support cases where user overrides finalize() and/or close() and Cleaner when he doesn't.
I wonder if it is the right thing to use AltFinalizer when user overrides finalize() method. In that case the method is probably not empty and calls super.finalize() (if it is empty or doesn't call super, user probably doesn't want the finalization to close the stream) and so normal finalization applies. If you register AltFinalizer for such case, close() will be called twice.
Ah, scrap that. I forgot that XXXStream.finalize() is now empty, so user overriding it and calling super does not in fact close the stream. You have to register AltFinalizer in that case. But now I wonder if the logic should still be 3-state and do the following:
- if user overrides finalize() - use AltFinalizer to call both: first finalize() and then close(); else - if user overrides close() - use AltFinalizer to call close(); else - use Cleaner
What do you think?
Regards, Peter
On 12/04/2017 02:25 PM, Peter Levart wrote:
Hi Rogger,
On 12/04/2017 02:17 PM, Peter Levart wrote:
Hi Rogger,
Interesting approach. Conditional finalization. You use finalization to support cases where user overrides finalize() and/or close() and Cleaner when he doesn't.
I wonder if it is the right thing to use AltFinalizer when user overrides finalize() method. In that case the method is probably not empty and calls super.finalize() (if it is empty or doesn't call super, user probably doesn't want the finalization to close the stream) and so normal finalization applies. If you register AltFinalizer for such case, close() will be called twice.
Ah, scrap that. I forgot that XXXStream.finalize() is now empty, so user overriding it and calling super does not in fact close the stream. You have to register AltFinalizer in that case. But now I wonder if the logic should still be 3-state and do the following:
- if user overrides finalize() - use AltFinalizer to call both: first finalize() and then close(); else - if user overrides close() - use AltFinalizer to call close(); else - use Cleaner
What do you think?
Regards, Peter
I just realized that in the above case when finalize is overridden, it would be called twice. once by finalization and once by AltFinalizer. So your logic is as correct as it can be for that case (to just call close() with AltFinalizer). The only problem is order which is arbitrary, so it may happen that AltFinalizer calls close() 1st and then finalization calls overridden finalize() method which might expect the stream to still be open until it calls super.finalize(). Regards, Peter
Hi Roger, On 12/04/2017 03:09 PM, Peter Levart wrote:
On 12/04/2017 02:25 PM, Peter Levart wrote:
Hi Rogger,
On 12/04/2017 02:17 PM, Peter Levart wrote:
Hi Rogger,
Interesting approach. Conditional finalization. You use finalization to support cases where user overrides finalize() and/or close() and Cleaner when he doesn't.
I wonder if it is the right thing to use AltFinalizer when user overrides finalize() method. In that case the method is probably not empty and calls super.finalize() (if it is empty or doesn't call super, user probably doesn't want the finalization to close the stream) and so normal finalization applies. If you register AltFinalizer for such case, close() will be called twice.
Ah, scrap that. I forgot that XXXStream.finalize() is now empty, so user overriding it and calling super does not in fact close the stream. You have to register AltFinalizer in that case. But now I wonder if the logic should still be 3-state and do the following:
- if user overrides finalize() - use AltFinalizer to call both: first finalize() and then close(); else - if user overrides close() - use AltFinalizer to call close(); else - use Cleaner
What do you think?
Regards, Peter
I just realized that in the above case when finalize is overridden, it would be called twice. once by finalization and once by AltFinalizer. So your logic is as correct as it can be for that case (to just call close() with AltFinalizer). The only problem is order which is arbitrary, so it may happen that AltFinalizer calls close() 1st and then finalization calls overridden finalize() method which might expect the stream to still be open until it calls super.finalize().
Regards, Peter
Final refinement... (hopefully!) If user overrides just finalize() and does not override close(), then it might be best to employ Cleaner to close the stream. Cleaner is Phantom based and will get fired after finalization invokes overridden finalize(), enabling the finalize() method to still access the stream in that case. So this is the final logic (I think): - if user overrides close(), use AltFinalizer to call close(); else - use Cleaner to close the stream The above logic in action: finalize() close() action not overridden not overridden Cleaner closes stream not overridden overridden AltFinalizer calls close() overridden not overridden finalization calls finalize() then Cleaner closes stream overridden overridden finalization calls finalize() and AltFinalizer calls close() (in arbitrary order) Regards, Peter
Hi Peter, Thanks for reviewing. This is a transition step to removing the finalize method completely while giving subclasses notice to upgrade their cleanup activities and yet gain the performance benefits sooner. Later, finalize() and related compatibility mechanisms will be removed. Simpler code, even if sometimes less than optimal is preferred to maintain compatibility in the interim. ... On 12/4/2017 9:25 AM, Peter Levart wrote:
Hi Roger,
On 12/04/2017 03:09 PM, Peter Levart wrote:
On 12/04/2017 02:25 PM, Peter Levart wrote:
Hi Rogger,
On 12/04/2017 02:17 PM, Peter Levart wrote:
Hi Rogger,
Interesting approach. Conditional finalization. You use finalization to support cases where user overrides finalize() and/or close() and Cleaner when he doesn't.
I wonder if it is the right thing to use AltFinalizer when user overrides finalize() method. In that case the method is probably not empty and calls super.finalize() (if it is empty or doesn't call super, user probably doesn't want the finalization to close the stream) and so normal finalization applies. If you register AltFinalizer for such case, close() will be called twice.
Ah, scrap that. I forgot that XXXStream.finalize() is now empty, so user overriding it and calling super does not in fact close the stream. You have to register AltFinalizer in that case. But now I wonder if the logic should still be 3-state and do the following:
- if user overrides finalize() - use AltFinalizer to call both: first finalize() and then close(); else - if user overrides close() - use AltFinalizer to call close(); else - use Cleaner
What do you think?
Regards, Peter
I just realized that in the above case when finalize is overridden, it would be called twice. once by finalization and once by AltFinalizer. So your logic is as correct as it can be for that case (to just call close() with AltFinalizer). The only problem is order which is arbitrary, so it may happen that AltFinalizer calls close() 1st and then finalization calls overridden finalize() method which might expect the stream to still be open until it calls super.finalize().
Regards, Peter
Final refinement... (hopefully!)
If user overrides just finalize() and does not override close(), then it might be best to employ Cleaner to close the stream. Cleaner is Phantom based and will get fired after finalization invokes overridden finalize(), enabling the finalize() method to still access the stream in that case. So this is the final logic (I think):
- if user overrides close(), use AltFinalizer to call close(); else - use Cleaner to close the stream
The above logic in action:
finalize() close() action
not overridden not overridden Cleaner closes stream not overridden overridden AltFinalizer calls close() overridden not overridden finalization calls finalize() then Cleaner closes stream overridden overridden finalization calls finalize() and AltFinalizer calls close() (in arbitrary order)
Right, if close() is not overridden the behavior of close() is known and the Cleaner can be used. The contents of an overridden finalize() method are unknowable, it may or may not call close() itself and may or may not call super.finalize(). If close() is called, then the Cleaner will be removed; otherwise it will release the resources. The close method should be idempotent, calling it twice should not be a problem. This does simplify the logic. Thanks, Roger
Regards, Peter
participants (2)
-
Peter Levart
-
Roger Riggs