Commons Dbcp
  1. Commons Dbcp
  2. DBCP-180

[dbcp] Dbcp doesn't meet JDBC specification

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 2.0
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: All

      Description

      The JDBC specification says for ResultSet and Statement that the ResultSet and
      Statement will be automatically closed when garbage collected.

      This means that people often write code like this

      Connection.getStatement().executeSQL("....");

      However, because the DelegateConnection/DelegateStatement/DelegateResultSet
      extend AbandonedTrace (and are linked into the abandoned trace mechanism) they
      will not be garbage collected until the connection is closed.

      This of course means that DBCP holds on to lots of memory that it shouldn't do
      (I've had people complain to me of memory leaks).

      Surely it's possible to make the DelegateXX hold onto something else which does
      the AbandonedTrace and so when it is released it can do the implicit close and
      then everything will be coool.

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        3408d 17h 38m 1 Mark Thomas 05/Feb/14 22:31
        Resolved Resolved Closed Closed
        383d 4h 51m 1 Phil Steitz 24/Feb/15 03:23
        Phil Steitz made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Mark Thomas made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Mark Thomas added a comment - - edited

        The issues that led to AbandonedTrace being deprecated were fixed and the deprecation removed.

        I have fixed this in DBCP 2 by using a WeakReference within AbandonedTrace.

        Show
        Mark Thomas added a comment - - edited The issues that led to AbandonedTrace being deprecated were fixed and the deprecation removed. I have fixed this in DBCP 2 by using a WeakReference within AbandonedTrace.
        Phil Steitz made changes -
        Fix Version/s 2.0 [ 12313721 ]
        Fix Version/s 1.4 [ 12312615 ]
        Phil Steitz made changes -
        Fix Version/s 1.4 [ 12312615 ]
        Fix Version/s 1.3 [ 12311977 ]
        Hide
        Phil Steitz added a comment -

        Absent a patch, move this to 1.4

        Show
        Phil Steitz added a comment - Absent a patch, move this to 1.4
        Phil Steitz made changes -
        Fix Version/s 1.3 [ 12311977 ]
        Bugzilla Id 31569
        Hide
        Phil Steitz added a comment -

        If a safe and efficient patch is submitted to enable this enhancement, we can include it in 1.3. Per Yoav's comment, the patch should not rely on AbandonedTrace.

        Show
        Phil Steitz added a comment - If a safe and efficient patch is submitted to enable this enhancement, we can include it in 1.3. Per Yoav's comment, the patch should not rely on AbandonedTrace.
        Henri Yandell made changes -
        Affects Version/s 1.2 Final [ 12311721 ]
        Henri Yandell made changes -
        Assignee Jakarta Commons Developers Mailing List [ commons-dev@jakarta.apache.org ]
        Key COM-1633 DBCP-180
        Affects Version/s 1.2 Final [ 12311661 ]
        Component/s Dbcp [ 12311109 ]
        Project Commons [ 12310458 ] Commons Dbcp [ 12310469 ]
        Henri Yandell made changes -
        Field Original Value New Value
        issue.field.bugzillaimportkey 31569 12341785
        Hide
        Rohan Lenard added a comment -

        (In reply to comment #1)
        > The JDBC Spec says indeed that those resources will be automatically closed
        > when garbag collected. It doesn't say when they can be garbage collected,
        nor
        > does it place restrictions on implementations to garbage collect after
        > executing SQL. So DBCP is not in violation of the Spec by a long shot.

        It also says
        "A ResultSet object is automatically closed when the Statement object that
        generated it is closed, re-executed, or used to retrieve the next result from a
        sequence of multiple results."

        and close says
        "Releases this ResultSet object's database and JDBC resources immediately
        instead of waiting for this to happen when it is automatically closed."

        Which is pretty clear that they should be release immediately - which DBCP
        doesn't do.

        > Accordingly, I'm changing this into an enhancement request. If you want to
        > increase the probability of it being done any time soon, submit a patch.

        I think you might have been a little hasty here - for the reason above.

        > item will become moot. Second, the best practice for JDBC resource handling
        is
        > to explicitly close what you don't need. People who write code as you
        > illustrate above are not following that practice and don't deserve much
        > sympathy.

        No argument here, except that "naive" users will use DBCP and NOT get the
        behaviour they expect !!

        Show
        Rohan Lenard added a comment - (In reply to comment #1) > The JDBC Spec says indeed that those resources will be automatically closed > when garbag collected. It doesn't say when they can be garbage collected, nor > does it place restrictions on implementations to garbage collect after > executing SQL. So DBCP is not in violation of the Spec by a long shot. It also says "A ResultSet object is automatically closed when the Statement object that generated it is closed, re-executed, or used to retrieve the next result from a sequence of multiple results." and close says "Releases this ResultSet object's database and JDBC resources immediately instead of waiting for this to happen when it is automatically closed." Which is pretty clear that they should be release immediately - which DBCP doesn't do. > Accordingly, I'm changing this into an enhancement request. If you want to > increase the probability of it being done any time soon, submit a patch. I think you might have been a little hasty here - for the reason above. > item will become moot. Second, the best practice for JDBC resource handling is > to explicitly close what you don't need. People who write code as you > illustrate above are not following that practice and don't deserve much > sympathy. No argument here, except that "naive" users will use DBCP and NOT get the behaviour they expect !!
        Hide
        Yoav Shapira added a comment -

        The JDBC Spec says indeed that those resources will be automatically closed
        when garbag collected. It doesn't say when they can be garbage collected, nor
        does it place restrictions on implementations to garbage collect after
        executing SQL. So DBCP is not in violation of the Spec by a long shot.

        Accordingly, I'm changing this into an enhancement request. If you want to
        increase the probability of it being done any time soon, submit a patch.

        Two other notes: First, AbandonedTrace is deprecated and has been for a while.
        It will likely be removed in the next major DBCP released, at which point this
        item will become moot. Second, the best practice for JDBC resource handling is
        to explicitly close what you don't need. People who write code as you
        illustrate above are not following that practice and don't deserve much
        sympathy.

        Show
        Yoav Shapira added a comment - The JDBC Spec says indeed that those resources will be automatically closed when garbag collected. It doesn't say when they can be garbage collected, nor does it place restrictions on implementations to garbage collect after executing SQL. So DBCP is not in violation of the Spec by a long shot. Accordingly, I'm changing this into an enhancement request. If you want to increase the probability of it being done any time soon, submit a patch. Two other notes: First, AbandonedTrace is deprecated and has been for a while. It will likely be removed in the next major DBCP released, at which point this item will become moot. Second, the best practice for JDBC resource handling is to explicitly close what you don't need. People who write code as you illustrate above are not following that practice and don't deserve much sympathy.
        Rohan Lenard created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Rohan Lenard
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development