Derby
  1. Derby
  2. DERBY-5960

VirtualRandomAccessFile.close() is not idempotent

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.10.1.1
    • Fix Version/s: 10.8.3.3, 10.9.2.2, 10.10.1.1
    • Component/s: Store
    • Labels:
      None

      Description

      If VirtualRandomAccessFile.close() is called more than once, which might happen in code that cleans up after errors, it will throw a NullPointerException. We should make close() idempotent so that it is more robust.

      1. d5960-1a.diff
        4 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          4h 28m 1 Knut Anders Hatlen 24/Oct/12 11:57
          In Progress In Progress Resolved Resolved
          1d 37m 1 Knut Anders Hatlen 25/Oct/12 12:35
          Resolved Resolved Closed Closed
          234d 20h 52m 1 Knut Anders Hatlen 17/Jun/13 09:27
          Kathey Marsden made changes -
          Assignee Kathey Marsden [ kmarsden ] Knut Anders Hatlen [ knutanders ]
          Fix Version/s 10.8.3.1 [ 12323475 ]
          Fix Version/s 10.9.2.2 [ 12323562 ]
          Hide
          ASF subversion and git services added a comment -

          Commit 1509794 from Kathey Marsden in branch 'code/branches/10.8'
          [ https://svn.apache.org/r1509794 ]

          DERBY-5960 VirtualRandomAccessFile.close() is not idempotent

          Merge revision 1402111 from trunk.
          Contributed by Knut Anders Hatlen

          Show
          ASF subversion and git services added a comment - Commit 1509794 from Kathey Marsden in branch 'code/branches/10.8' [ https://svn.apache.org/r1509794 ] DERBY-5960 VirtualRandomAccessFile.close() is not idempotent Merge revision 1402111 from trunk. Contributed by Knut Anders Hatlen
          Hide
          ASF subversion and git services added a comment -

          Commit 1509763 from Kathey Marsden in branch 'code/branches/10.9'
          [ https://svn.apache.org/r1509763 ]

          DERBY-5960 VirtualRandomAccessFile.close() is not idempotent

          Merge revision 1402111 from trunk.
          Contributed by Knut Anders Hatlen

          Show
          ASF subversion and git services added a comment - Commit 1509763 from Kathey Marsden in branch 'code/branches/10.9' [ https://svn.apache.org/r1509763 ] DERBY-5960 VirtualRandomAccessFile.close() is not idempotent Merge revision 1402111 from trunk. Contributed by Knut Anders Hatlen
          Kathey Marsden made changes -
          Assignee Knut Anders Hatlen [ knutanders ] Kathey Marsden [ kmarsden ]
          Hide
          Kathey Marsden added a comment -

          temporarily reassign for backport.

          Show
          Kathey Marsden added a comment - temporarily reassign for backport.
          Gavin made changes -
          Workflow jira [ 12731157 ] Default workflow, editable Closed status [ 12802064 ]
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Knut Anders Hatlen made changes -
          Issue & fix info Patch Available [ 10102 ]
          Status In Progress [ 3 ] Resolved [ 5 ]
          Fix Version/s 10.10.0.0 [ 12321550 ]
          Resolution Fixed [ 1 ]
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1402111.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1402111.
          Knut Anders Hatlen made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Knut Anders Hatlen made changes -
          Issue & fix info Patch Available [ 10102 ]
          Knut Anders Hatlen made changes -
          Attachment d5960-1a.diff [ 12550623 ]
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a fix, including a test case in VirtualFileTest.

          The patch simply makes close() stop nulling out the fields so that it won't run into a NPE on subsequent executions.

          (I preferred that approach to adding extra null checks in close(), as it allows the code to be slightly shorter/simpler. The advantage of nulling the fields and adding extra checks would be that the objects references by the fields would be eligible for gc earlier if the VirtualRandomAccessFile instance itself was still referenced after close(). I believe that this is not the case in normal use of VRAF. And even if it is used that way, there's not much to gain since the big data structures (the byte arrays) in those instances are already freed. So adding extra code to optimize for that case didn't seem worthwhile.)

          I also used the opportunity to fix a typo in VirtualFile and make some of the fields in VirtualRandomAccessFile final.

          All the regression tests ran cleanly with the patch.

          Show
          Knut Anders Hatlen added a comment - Attaching a fix, including a test case in VirtualFileTest. The patch simply makes close() stop nulling out the fields so that it won't run into a NPE on subsequent executions. (I preferred that approach to adding extra null checks in close(), as it allows the code to be slightly shorter/simpler. The advantage of nulling the fields and adding extra checks would be that the objects references by the fields would be eligible for gc earlier if the VirtualRandomAccessFile instance itself was still referenced after close(). I believe that this is not the case in normal use of VRAF. And even if it is used that way, there's not much to gain since the big data structures (the byte arrays) in those instances are already freed. So adding extra code to optimize for that case didn't seem worthwhile.) I also used the opportunity to fix a typo in VirtualFile and make some of the fields in VirtualRandomAccessFile final. All the regression tests ran cleanly with the patch.
          Knut Anders Hatlen made changes -
          Field Original Value New Value
          Link This issue relates to DERBY-5232 [ DERBY-5232 ]
          Knut Anders Hatlen created issue -

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development