Derby
  1. Derby
  2. DERBY-4717

Driver trace file isn't closed/released on physical connection close when specified with the traceFile attribute/setter

    Details

      Description

      When specifying a trace file with the traceFile connection URL attribute or with the data source setter method, the handle to the trace file isn't released when the physical connection is closed.
      The problem may be specific to the implementations of ConnectionPoolDataSource and XADataSource, as using a plain data source doesn't involve reuse (i.e another mechanism may close the trace file handle).

      Two potential problems caused by this bug:
      o resource leak (i.e., too many file handles open)
      o trace files cannot be deleted on Windows (seen for DERBY-4709)

      1. derby-4717-1b.diff
        10 kB
        Kristian Waagan
      2. derby.log
        66 kB
        Lily Wei
      3. derby-4717-1a.diff
        10 kB
        Kristian Waagan

        Issue Links

          Activity

          Hide
          Kristian Waagan added a comment -

          Attaching patch 1a, which consists of the following changes:

          (production code)
          a) Make Derby close the trace file when the physical connection is closed and the trace file is specified using 'traceFile' (attribute or setter).
          (test code)
          b) Disabled the test 'testClientMessageTextConnectionAttribute', as the test started failing when I corrected the test. It was using (ConnectionPool|XA)DataSource.getConnection() to obtain a connection. This method is not an API method, it's an artifact due to Derby's subclassing (the two mentioned data sources extend ClientBaseDataSource). DERBY-4067 trace the problem that the connection attributes are not picked up.
          c) Made the tests obtain the physical connections from data source by using the correct method (i.e. getXAConnection, getPooledConnection or getConnection).
          d) Extended the test 'testClientTraceFileDSConnectionAttribute' to also make sure the trace files can be deleted (regression test for DERBY-4717).
          e) Added 'getPhysicalConnection(Object ds)' for the cases where a physical connection is to be obtained, but the data source can be of any (Derby) type.

          I need someone to test this patch on Windows, i.e. that jdbcapi.J2EEDataSourceTest.testClientTraceFileDSConnectionAttribute fails without the changes to ClientBaseDataSource.

          Patch ready for review.

          Show
          Kristian Waagan added a comment - Attaching patch 1a, which consists of the following changes: (production code) a) Make Derby close the trace file when the physical connection is closed and the trace file is specified using 'traceFile' (attribute or setter). (test code) b) Disabled the test 'testClientMessageTextConnectionAttribute', as the test started failing when I corrected the test. It was using (ConnectionPool|XA)DataSource.getConnection() to obtain a connection. This method is not an API method, it's an artifact due to Derby's subclassing (the two mentioned data sources extend ClientBaseDataSource). DERBY-4067 trace the problem that the connection attributes are not picked up. c) Made the tests obtain the physical connections from data source by using the correct method (i.e. getXAConnection, getPooledConnection or getConnection). d) Extended the test 'testClientTraceFileDSConnectionAttribute' to also make sure the trace files can be deleted (regression test for DERBY-4717 ). e) Added 'getPhysicalConnection(Object ds)' for the cases where a physical connection is to be obtained, but the data source can be of any (Derby) type. I need someone to test this patch on Windows, i.e. that jdbcapi.J2EEDataSourceTest.testClientTraceFileDSConnectionAttribute fails without the changes to ClientBaseDataSource. Patch ready for review.
          Hide
          Lily Wei added a comment -

          Apply the patch and test it on Windows. With or without ClienBaseDataSource changes, the tests both failed with not privilege on delete data files on assertTraceFilesExistAndCanBeDeleted

          [Error/failure logged at Mon Jun 28 13:04:48 PDT 2010]
          junit.framework.AssertionFailedError: Delete failed
          at junit.framework.Assert.fail(Assert.java:47)
          at junit.framework.Assert.assertTrue(Assert.java:20)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTest$8.run(J2EEDataSour
          ceTest.java:2695)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTest.assertTraceFilesEx
          istAndCanBeDeleted(J2EEDataSourceTest.java:2685)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTest.testClientTraceFil
          eDSConnectionAttribute(J2EEDataSourceTest.java:2675)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
          at java.lang.reflect.Method.invoke(Unknown Source)
          at junit.framework.TestCase.runTest(TestCase.java:154)
          at junit.framework.TestCase.runBare(TestCase.java:127)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:109)
          at junit.framework.TestResult$1.protect(TestResult.java:106)
          at junit.framework.TestResult.runProtected(TestResult.java:124)
          at junit.framework.TestResult.run(TestResult.java:109)
          at junit.framework.TestCase.run(TestCase.java:118)

          Show
          Lily Wei added a comment - Apply the patch and test it on Windows. With or without ClienBaseDataSource changes, the tests both failed with not privilege on delete data files on assertTraceFilesExistAndCanBeDeleted [Error/failure logged at Mon Jun 28 13:04:48 PDT 2010] junit.framework.AssertionFailedError: Delete failed at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.assertTrue(Assert.java:20) at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTest$8.run(J2EEDataSour ceTest.java:2695) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTest.assertTraceFilesEx istAndCanBeDeleted(J2EEDataSourceTest.java:2685) at org.apache.derbyTesting.functionTests.tests.jdbcapi.J2EEDataSourceTest.testClientTraceFil eDSConnectionAttribute(J2EEDataSourceTest.java:2675) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at junit.framework.TestCase.runTest(TestCase.java:154) at junit.framework.TestCase.runBare(TestCase.java:127) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:109) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:118)
          Hide
          Kristian Waagan added a comment -

          Thanks, Lily.

          Attaching revision 1b, which hopefully fixes the problem (seen on Windows).
          The existing test didn't close the connection, and I forgot to change it.

          Show
          Kristian Waagan added a comment - Thanks, Lily. Attaching revision 1b, which hopefully fixes the problem (seen on Windows). The existing test didn't close the connection, and I forgot to change it.
          Hide
          Lily Wei added a comment -

          Thanks, Kristian and Dag. The test passed for me on Windows. Yeah!!!

          Show
          Lily Wei added a comment - Thanks, Kristian and Dag. The test passed for me on Windows. Yeah!!!
          Hide
          Kristian Waagan added a comment -

          Thanks, Dag and Lily.

          Committed and backported:

          • trunk: 959550
          • 10.6 : 959552
          • 10.5 : 959553
          • 10.4 : 959554
          Show
          Kristian Waagan added a comment - Thanks, Dag and Lily. Committed and backported: trunk: 959550 10.6 : 959552 10.5 : 959553 10.4 : 959554

            People

            • Assignee:
              Kristian Waagan
              Reporter:
              Kristian Waagan
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development