Fwd: Re: RFR 8080225 FileInput/OutputStream/FileChannel cleanup should be improved (original) (raw)

Roger Riggs Roger.Riggs at Oracle.com
Mon Dec 4 16:58:21 UTC 2017

Hi Peter,

I'd like to keep the code recognizably simple; (in the absence of a known performance issue). It would be interesting to know how the streams versions compares to the exception versions. Something for a rainy day...

Roger

On 12/4/2017 8:29 AM, Peter Levart wrote:

Sending previous message to the list also (forgot to mention it in CC)...

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. The logic should probably be 3-state: - if user overrides finalize() - do nothing; else - if user overrides close() - use AltFinalizer; else - use Cleaner Some additional comments: - FileInputStream.AltFinalizer#get could be written to not use exceptions for flow control. For example: static AltFinalizer get(FileInputStream fis) { return Stream.<Class<?>>iterate(fis.getClass(), cl -> cl != FileInputStream.class, Class::getSuperclass) .flatMap(clazz -> Stream.of(clazz.getDeclaredMethods())) .filter(m -> m.getParameterCount() == 0 && (m.getName().equals("close") || m.getName().equals("finalize"))) .findFirst() .map(m -> new AltFinalizer(fis)).orElse(null); } or, to support 3-state logic: static Optional get(FileInputStream fis) { int flag = Stream. <Class<?>>iterate(fis.getClass(), cl -> cl != FileInputStream.class, Class::getSuperclass) .flatMap(clazz -> Stream.of(clazz.getDeclaredMethods())) .mapToInt(m -> m.getParameterCount() == 0 ? m.getName().equals("close") ? 1 : (m.getName().equals("finalize") ? 2 : 0) : 0) .reduce(0, (i, j) -> i | j); if ((flag & 2) != 0) { // overrides finalize() return Optional.empty(); } else if ((flag & 1) != 0) { // overrides close() return Optional.of(new AltFinalizer(fis)); } else { // overrides none return null; } } - similar for FileOutputStream.AltFinalizer

That's all for now. Will try to check the rest later. Regards, Peter On 12/02/2017 03:38 AM, Roger Riggs wrote: Please review a revision to the work on remove a dependency on finalization from FileInputStream, FileOutputStream, and add cleanup of closing file descriptors in FileChannel.

The previous version was too aggressive in removing the finalize method and was considered to be insufficiently backward compatible. For compatibility with previous versions, the close method will be called when the FIS/FOS is unreachable only when FIS/FOS has been subclassed to override close() or finalize().  In other cases, the cleaner is used to make sure the FileDescriptor is closed. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-fis-cleanup-8080225/ Issue: https://bugs.openjdk.java.net/browse/JDK-8080225 CSR: Relax FileInputStream/FileOutputStream requirement to use finalize https://bugs.openjdk.java.net/browse/JDK-8187325 Thanks, Roger

More information about the core-libs-dev mailing list