Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8187325

Relax FileInputStream/FileOutputStream requirement to use finalize

    Details

    • Type: CSR
    • Status: Closed
    • Priority: P3
    • Resolution: Approved
    • Fix Version/s: 10
    • Component/s: core-libs
    • Labels:
      None
    • Subcomponent:
    • Compatibility Kind:
      behavioral
    • Compatibility Risk:
      low
    • Compatibility Risk Description:
      Hide
      The change to clarify that calling `finalize` can not be expected to call `close` should have a negligible effect. The recommended use to free resources is to call close. If not closed explicitly, the resources will be released when the FIS/FOS becomes unreachable.
      Show
      The change to clarify that calling `finalize` can not be expected to call `close` should have a negligible effect. The recommended use to free resources is to call close. If not closed explicitly, the resources will be released when the FIS/FOS becomes unreachable.
    • Interface Kind:
      Java API
    • Scope:
      SE

      Description

      Summary

      FileInputStream and FileOutputStream and subclasses should have more flexibility to implement the closing of FileDescriptors and release other resources when the streams are unreferenced. Requiring use of finalization in all cases has a negative performance impact.

      Problem

      The use of finalization by FIS/FOS to close file descriptors adds a significant overhead to garbage collection and reference processing of finalizable references.

      The specification of FileInputStream and FileOutputStream finalize methods requires the close method to be called when "there are no more references to it". As written implies, but does not mandate that finalize calls close though that is the current implementation. A clarification can enable a more efficient implementation.

      Also, the deprecation of the finalize methods should include forRemoval = true to give adequate notice of future removal of the finalize method.

      Solution

      The specifications of FileInputStream and FileOutputStream are changed to require the calling of close when "there are no more references to it" only in the case where it would affect a subclass that has overridden either 'finalize' or 'close'. It is also explicit that finalize does not call close directly. In other cases, the release of resources is implementation specific.

      This conditional use of finalization allows the implementation flexibility to use finalization (but not necessarily the finalize method) to call close in cases where it may affect subclass behavior. Compatibility with current behavior is maintained except for the case where finalize is called directly, which is not usually the case.

      12/5/2017: Update to simplify the condition under which the close method is called to depend only on the close method being overridden. The finalize method is not longer included in the condition.

      Specification

      java.io.FileInputStream

      Class javadoc:

      + * @apiNote
      + * To release resources used by this stream {@linkplain #close} should be called
      + * directly or by try-with-resources. Subclasses are responsible for the cleanup
      + * of resources acquired by the subclass.
      + * Subclasses that override {@link #finalize} in order to perform cleanup
      + * should be modified to use alternative cleanup mechanisms such as
      + * {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method.
      + *
      + * @implSpec
      + * If this FileInputStream has been subclassed and the {@linkplain #close} 
      + * method has been overridden, the {@linkplain #close} method will be
      + * called when the FileInputStream is unreachable.
      + * Otherwise, it is implementation specific how the resource cleanup described in
      + * {@linkplain #close} is performed.
      

      Method finalize():

           /**
            * Ensures that the {@linkplain #close} method of this file input stream is
            * called when there are no more references to it.
      +     * The {@link #finalize} method does not call {@linkplain #close} directly.
            *
      +     * @implSpec
      +     * If this FileInputStream has been subclassed and the {@linkplain #close}
      +     * method has been overridden, the {@linkplain #close} method will be
      +     * called when the FileInputStream is unreachable.
      +     * Otherwise, it is implementation specific how the resource cleanup described in
      +     * {@linkplain #close} is performed.
      +     *
      +     * @apiNote
      +     * To release resources used by this stream {@linkplain #close} should be called
      +     * directly or by try-with-resources.
      +     *
      +     * @deprecated The {@code finalize} method has been deprecated and will be removed.
            *     Subclasses that override {@code finalize} in order to perform cleanup
            *     should be modified to use alternative cleanup mechanisms and
            *     to remove the overriding {@code finalize} method.
      @@ -425,15 +460,39 @@
            * @exception  IOException  if an I/O error occurs.
            * @see        java.io.FileInputStream#close()
            */
      +    @Deprecated(since="9", forRemoval = true) {...}

      java.io.FileOutputStream

      Class javadoc:

      + * @apiNote
      + * To release resources used by this stream {@linkplain #close} should be called
      + * directly or by try-with-resources. Subclasses are responsible for the cleanup
      + * of resources acquired by the subclass.
      + * Subclasses that override {@link #finalize} in order to perform cleanup
      + * should be modified to use alternative cleanup mechanisms such as
      + * {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method.
      + *
      + * @implSpec
      + * If this FileOutputStream has been subclassed and the {@linkplain #close}
      + * method has been overridden, the {@linkplain #close} method will be
      + * called when the FileOutputStream is unreachable.
      + * Otherwise, it is implementation specific how the resource cleanup described in
      + * {@linkplain #close} is performed.

      Method finalize():

           /**
            * Cleans up the connection to the file, and ensures that the
            * {@linkplain #close} method of this file output stream is
            * called when there are no more references to this stream.
      +     * The {@link #finalize} method does not call {@linkplain #close} directly.
      +     *
      +     * @implSpec
      +     * If this FileOutputStream has been subclassed and the {@linkplain #close}
      +     * method has been overridden, the {@linkplain #close} method will be
      +     * called when the FileOutputStream is unreachable.
      +     * Otherwise, it is implementation specific how the resource cleanup described in
      +     * {@linkplain #close} is performed.
      +     *
      +     * @apiNote
      +     * To release resources used by this stream {@linkplain #close} should be called
      +     * directly or by try-with-resources.
      +     *
      +     * @deprecated The {@code finalize} method has been deprecated and will be removed.
      +     *     Subclasses that override {@code finalize} in order to perform cleanup
      +     *     should be modified to use alternative cleanup mechanisms and
      +     *     to remove the overriding {@code finalize} method.
      +     *     When overriding the {@code finalize} method, its implementation must explicitly
      +     *     ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}.
      +     *     See the specification for {@link Object#finalize()} for further
      +     *     information about migration options.
            *
            * @exception  IOException  if an I/O error occurs.
            * @see        java.io.FileInputStream#close()
            */
      +    @Deprecated(since="9", forRemoval = true)
           protected void finalize() throws IOException {...}

        Issue Links

          Activity

          Hide
          darcy Joe Darcy added a comment -

          Sorry for the belated review.

          Upon considering the merits of the proposal and the age of the classes in question, dating all the way back to 1.0, I think the current proposal is too aggressive in removing the finalize methods in JDK 10.

          I'd prefer to see a staged approach were the finalize methods are changed from being traditionally deprecated to deprecated-for-removal but with the same behavioral semantics as today. Also, subclasses should be given more precise guidance on how to clean up their own state without relying on close called via finalize.

          Marking the request as pended.

          Show
          darcy Joe Darcy added a comment - Sorry for the belated review. Upon considering the merits of the proposal and the age of the classes in question, dating all the way back to 1.0, I think the current proposal is too aggressive in removing the finalize methods in JDK 10. I'd prefer to see a staged approach were the finalize methods are changed from being traditionally deprecated to deprecated-for-removal but with the same behavioral semantics as today. Also, subclasses should be given more precise guidance on how to clean up their own state without relying on close called via finalize. Marking the request as pended.
          Hide
          rriggs Roger Riggs added a comment -

          For clarification, the recommendation is to not make any changes to the specification or behavior except to mark finalize for removal and provide additional guidance. So a change to remove the requirement that the finalize method calls close needs to be deferred. (There is a different implementation technique that would call close when it was finalized).

          Show
          rriggs Roger Riggs added a comment - For clarification, the recommendation is to not make any changes to the specification or behavior except to mark finalize for removal and provide additional guidance. So a change to remove the requirement that the finalize method calls close needs to be deferred. (There is a different implementation technique that would call close when it was finalized).
          Hide
          darcy Joe Darcy added a comment -

          @Roger, right; for JDK 10 I recommend:

          • Upgrading (downgrading?) the deprecation status of these methods to @Deprecated(forRemoval=true)

          • Telling diligent subclasses what they should do now to make their code work once the finalize methods are actually removed. Basically, how to write a subclass that has proper semantics both with the current FileInputStream/FileOutputStream and the anticipated future FileInputStream/FileOutputStream.

          The eventual remove could potentially be done in stages, stage 1 removing the method body but leaving a trivial override to avoid the source incompatibility and stage 2 being full removal.

          Show
          darcy Joe Darcy added a comment - @Roger, right; for JDK 10 I recommend: Upgrading (downgrading?) the deprecation status of these methods to @Deprecated(forRemoval=true) Telling diligent subclasses what they should do now to make their code work once the finalize methods are actually removed. Basically, how to write a subclass that has proper semantics both with the current FileInputStream/FileOutputStream and the anticipated future FileInputStream/FileOutputStream. The eventual remove could potentially be done in stages, stage 1 removing the method body but leaving a trivial override to avoid the source incompatibility and stage 2 being full removal.
          Hide
          darcy Joe Darcy added a comment -

          A reminder, the CSR request will stay in a pended state unless there is more discussion or a revised proposal from the CSR assignee.

          Show
          darcy Joe Darcy added a comment - A reminder, the CSR request will stay in a pended state unless there is more discussion or a revised proposal from the CSR assignee.
          Hide
          alanb Alan Bateman added a comment -

          I don't think Joe's suggestion to add @Deprecated(forRemoval=true) will help much as the issue is classes that extend FIS/FOS and override the close method (not the finalize method).

          Show
          alanb Alan Bateman added a comment - I don't think Joe's suggestion to add @Deprecated(forRemoval=true) will help much as the issue is classes that extend FIS/FOS and override the close method (not the finalize method).
          Hide
          rriggs Roger Riggs added a comment -

          The proposal has been significantly revised to describe a change in behavior that can enable a more efficient cleanup while maintaining compatibility except for a narrow special case. Please reconsider and review.

          Show
          rriggs Roger Riggs added a comment - The proposal has been significantly revised to describe a change in behavior that can enable a more efficient cleanup while maintaining compatibility except for a narrow special case. Please reconsider and review.
          Hide
          darcy Joe Darcy added a comment -

          I'm voting to approve this request subject to a follow-up bug for removing the finalize method be created and linked to the issue.

          As suggested in discussion of this matter outside of the CSR, it may be helpful to augment the set of warnings produced by jdeprscan to include the close method of subclasses of FIS/FOS and similar situations.

          Show
          darcy Joe Darcy added a comment - I'm voting to approve this request subject to a follow-up bug for removing the finalize method be created and linked to the issue. As suggested in discussion of this matter outside of the CSR, it may be helpful to augment the set of warnings produced by jdeprscan to include the close method of subclasses of FIS/FOS and similar situations.
          Hide
          rriggs Roger Riggs added a comment -

          Based on review comments, the spec is updated to conditionally call close() when the stream is unreachable only in the case where close is overridden. The changes are to the class javadoc @implSpec and method javadoc for finalize

          Show
          rriggs Roger Riggs added a comment - Based on review comments, the spec is updated to conditionally call close() when the stream is unreachable only in the case where close is overridden. The changes are to the class javadoc @implSpec and method javadoc for finalize
          Hide
          darcy Joe Darcy added a comment -

          Moving to Approved.

          Show
          darcy Joe Darcy added a comment - Moving to Approved.

            People

            • Assignee:
              rriggs Roger Riggs
              Reporter:
              rriggs Roger Riggs
              Reviewed By:
              Alan Bateman, Brian Burkhalter, Chris Hegarty
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: