Derby
  1. Derby
  2. DERBY-4669

ClassLoaderBootTest fails if derbyclient.jar comes before derby.jar on the classpath

    Details

    • Urgency:
      Normal
    • Bug behavior facts:
      Regression Test Failure

      Description

      If derbyclient.jar comes before derby.jar on the classpath, and the build is sane, the test fails.

      java -cp derbyclient.jar:derby.jar:derbyTesting.jar:junit.jar junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest
      ...
      1) testBootingAnAlreadyBootedDatabase(org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest)java.lang.ExceptionInInitializerError
      at org.apache.derby.jdbc.EmbeddedDataSource.findDriver(EmbeddedDataSource.java:500)
      at org.apache.derby.jdbc.EmbeddedDataSource.getConnection(EmbeddedDataSource.java:479)
      at org.apache.derby.jdbc.EmbeddedDataSource.getConnection(EmbeddedDataSource.java:423)
      at org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest.testBootingAnAlreadyBootedDatabase(ClassLoaderBootTest.java:178)
      at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:109)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      at junit.extensions.TestSetup.run(TestSetup.java:25)
      at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      at junit.extensions.TestSetup.run(TestSetup.java:25)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      at junit.extensions.TestSetup.run(TestSetup.java:25)
      Caused by: java.lang.SecurityException: sealing violation: package org.apache.derby.iapi.services.sanity is sealed
      at java.net.URLClassLoader.defineClass(URLClassLoader.java:234)
      at java.net.URLClassLoader.access$000(URLClassLoader.java:58)
      at java.net.URLClassLoader$1.run(URLClassLoader.java:197)
      at java.security.AccessController.doPrivileged(Native Method)
      at java.net.URLClassLoader.findClass(URLClassLoader.java:190)
      at org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest$DerbyURLClassLoader.loadClass(ClassLoaderBootTest.java:293)
      at org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest$DerbyURLClassLoader.loadClass(ClassLoaderBootTest.java:303)
      at org.apache.derby.iapi.services.monitor.Monitor.startSystemModule(Monitor.java:369)
      at org.apache.derby.impl.services.monitor.BaseMonitor.runWithState(BaseMonitor.java:386)
      at org.apache.derby.impl.services.monitor.FileMonitor.<init>(FileMonitor.java:60)
      at org.apache.derby.iapi.services.monitor.Monitor.startMonitor(Monitor.java:289)
      at org.apache.derby.iapi.jdbc.JDBCBoot.boot(JDBCBoot.java:69)
      at org.apache.derby.jdbc.EmbeddedDriver.boot(EmbeddedDriver.java:199)
      at org.apache.derby.jdbc.EmbeddedDriver.<clinit>(EmbeddedDriver.java:96)
      ... 33 more
      2) testBootingDatabaseShutdownByAnotherCLR(org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest)java.lang.ExceptionInInitializerError
      at org.apache.derby.jdbc.EmbeddedDataSource.findDriver(EmbeddedDataSource.java:500)
      at org.apache.derby.jdbc.EmbeddedDataSource.getConnection(EmbeddedDataSource.java:479)
      at org.apache.derby.jdbc.EmbeddedDataSource.getConnection(EmbeddedDataSource.java:423)
      at org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest.testBootingDatabaseShutdownByAnotherCLR(ClassLoaderBootTest.java:208)
      at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:109)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      at junit.extensions.TestSetup.run(TestSetup.java:25)
      at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      at junit.extensions.TestSetup.run(TestSetup.java:25)
      at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
      at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
      at junit.extensions.TestSetup.run(TestSetup.java:25)
      Caused by: java.lang.SecurityException: sealing violation: package org.apache.derby.iapi.services.sanity is sealed
      at java.net.URLClassLoader.defineClass(URLClassLoader.java:234)
      at java.net.URLClassLoader.access$000(URLClassLoader.java:58)
      at java.net.URLClassLoader$1.run(URLClassLoader.java:197)
      at java.security.AccessController.doPrivileged(Native Method)
      at java.net.URLClassLoader.findClass(URLClassLoader.java:190)
      at org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest$DerbyURLClassLoader.loadClass(ClassLoaderBootTest.java:293)
      at org.apache.derbyTesting.functionTests.tests.store.ClassLoaderBootTest$DerbyURLClassLoader.loadClass(ClassLoaderBootTest.java:303)
      at org.apache.derby.iapi.services.monitor.Monitor.startSystemModule(Monitor.java:369)
      at org.apache.derby.impl.services.monitor.BaseMonitor.runWithState(BaseMonitor.java:386)
      at org.apache.derby.impl.services.monitor.FileMonitor.<init>(FileMonitor.java:60)
      at org.apache.derby.iapi.services.monitor.Monitor.startMonitor(Monitor.java:289)
      at org.apache.derby.iapi.jdbc.JDBCBoot.boot(JDBCBoot.java:69)
      at org.apache.derby.jdbc.EmbeddedDriver.boot(EmbeddedDriver.java:199)
      at org.apache.derby.jdbc.EmbeddedDriver.<clinit>(EmbeddedDriver.java:96)
      ... 33 more

      1. derby-4669-3a-unseal_common.stat
        0.2 kB
        Kristian Waagan
      2. derby-4669-3a-unseal_common.diff
        3 kB
        Kristian Waagan
      3. derby-4669-2b-unseal_iapi_sanity.diff
        0.6 kB
        Kristian Waagan
      4. derby-4669-2a-unseal_iapi_sanity.diff
        0.5 kB
        Kristian Waagan
      5. client-imports.diff
        3 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          I think this is because some client classes use org.apache.derby.iapi.services.sanity.SanityManager so that that class is included both in derby.jar and in derbyclient.jar. The client classes should use the SanityManager class in org.apache.derby.shared.common.sanity instead.

          Show
          Knut Anders Hatlen added a comment - I think this is because some client classes use org.apache.derby.iapi.services.sanity.SanityManager so that that class is included both in derby.jar and in derbyclient.jar. The client classes should use the SanityManager class in org.apache.derby.shared.common.sanity instead.
          Hide
          Knut Anders Hatlen added a comment -

          The attached patch (client-imports.diff) makes four client classes use SanityManager from shared.common instead of iapi.services, just by changing some import statements. This change removes the org.apache.derby.iapi.services.sanity package from the sane version of derbyclient.jar.

          This is however only a partial fix. ClassLoaderBootTest still fails, but now with a slightly different error message:

          Caused by: java.lang.SecurityException: sealing violation: package org.apache.derby.shared.common.sanity is sealed

          It looks like classes in the common.sanity package are included in both derbyclient.jar and derby.jar. I think this is because

          1) Some of the engine classes use SanityManager from common instead of iapi

          2) SanityManager and AssertFailure in iapi extend classes in common

          Show
          Knut Anders Hatlen added a comment - The attached patch (client-imports.diff) makes four client classes use SanityManager from shared.common instead of iapi.services, just by changing some import statements. This change removes the org.apache.derby.iapi.services.sanity package from the sane version of derbyclient.jar. This is however only a partial fix. ClassLoaderBootTest still fails, but now with a slightly different error message: Caused by: java.lang.SecurityException: sealing violation: package org.apache.derby.shared.common.sanity is sealed It looks like classes in the common.sanity package are included in both derbyclient.jar and derby.jar. I think this is because 1) Some of the engine classes use SanityManager from common instead of iapi 2) SanityManager and AssertFailure in iapi extend classes in common
          Hide
          Knut Anders Hatlen added a comment -

          All the regression tests ran cleanly with client-imports.diff. Committed revision 946641.

          Show
          Knut Anders Hatlen added a comment - All the regression tests ran cleanly with client-imports.diff. Committed revision 946641.
          Hide
          Knut Anders Hatlen added a comment -

          As to the engine problems, (1) should be easy to solve the same way as we did on the client. Not sure what's the best solution for (2).

          I don't think it was intentional when the shared.common.sanity variant of the SanityManager was introduced, that the iapi SanityManager should depend on it and need those classes in the jar file. If I remember the discussion about sharing code between the client and the engine correctly, it was said that it was OK to avoid duplicating constants by inheriting interfaces, because references to constants in interfaces will be inlined at compile-time and not introduce run-time dependencies. Classes and methods, on the other hand, would have to be duplicated, so that mixing different versions on the classpath would not cause problems.

          We could fix (2) by duplicating the code in the shared classes instead of using inheritance, and thereby making sure derby.jar doesn't contain the shared package. That would be more in line with the intention of the shared package, I believe. However, we may not be as worried about possible problems with mixed versions of debug jars as we are with insane production jars. So perhaps it would be acceptable simply to unseal the shared.common.sanity package.

          Show
          Knut Anders Hatlen added a comment - As to the engine problems, (1) should be easy to solve the same way as we did on the client. Not sure what's the best solution for (2). I don't think it was intentional when the shared.common.sanity variant of the SanityManager was introduced, that the iapi SanityManager should depend on it and need those classes in the jar file. If I remember the discussion about sharing code between the client and the engine correctly, it was said that it was OK to avoid duplicating constants by inheriting interfaces, because references to constants in interfaces will be inlined at compile-time and not introduce run-time dependencies. Classes and methods, on the other hand, would have to be duplicated, so that mixing different versions on the classpath would not cause problems. We could fix (2) by duplicating the code in the shared classes instead of using inheritance, and thereby making sure derby.jar doesn't contain the shared package. That would be more in line with the intention of the shared package, I believe. However, we may not be as worried about possible problems with mixed versions of debug jars as we are with insane production jars. So perhaps it would be acceptable simply to unseal the shared.common.sanity package.
          Hide
          Kristian Waagan added a comment -

          I'm getting fed up with this error, as it has now popped up again after I started using a new machine. The classpath is constructed using a 'for j in `ls *jar`' loop, and derbyclient.jar ends up before derby.jar. It's annoying to work around this, and it is also a bad issue to hit for newcomers running the tests.

          I have logged DERBY-5253 for (1), and I'm uploading patch 2a to unseal the iapi.sanity package to address (2). The manifest entry will be present in insane (production) jars too, I don't know if that is an issue?

          Does anyone oppose to unseal the iapi.sanity package?

          Show
          Kristian Waagan added a comment - I'm getting fed up with this error, as it has now popped up again after I started using a new machine. The classpath is constructed using a 'for j in `ls *jar`' loop, and derbyclient.jar ends up before derby.jar. It's annoying to work around this, and it is also a bad issue to hit for newcomers running the tests. I have logged DERBY-5253 for (1), and I'm uploading patch 2a to unseal the iapi.sanity package to address (2). The manifest entry will be present in insane (production) jars too, I don't know if that is an issue? Does anyone oppose to unseal the iapi.sanity package?
          Hide
          Knut Anders Hatlen added a comment -

          Unsealing the sanity package sounds fine to me, as it should have no consequences for the production jars (except the extra entry in the manifest, but that's not a big deal, I think).

          Show
          Knut Anders Hatlen added a comment - Unsealing the sanity package sounds fine to me, as it should have no consequences for the production jars (except the extra entry in the manifest, but that's not a big deal, I think).
          Hide
          Kristian Waagan added a comment -

          Attached patch 2b, which adds a comment and removes the tab used for indentation.

          Committed patch 2b to trunk with revision 1130084.

          I'll let this issue rest for a while, and then I plan to back-port (10.7 and 10.8) it if there are no objections or problems caused by the change.

          Show
          Kristian Waagan added a comment - Attached patch 2b, which adds a comment and removes the tab used for indentation. Committed patch 2b to trunk with revision 1130084. I'll let this issue rest for a while, and then I plan to back-port (10.7 and 10.8) it if there are no objections or problems caused by the change.
          Hide
          Kristian Waagan added a comment -

          I unsealed the wrong package in patch 2b, so here comes patch 3a...

          Patch 3a does the following:
          o replaces an import of iapi.sanity.SanityManager in a client class
          o changes an import to common.shared.sanity.SanityManager in the engine code. This breaks the current pattern, but makes it possible to unseal only the shared.common.sanity package. The reason is that the class JVMInfo is included in both the engine and the client code.
          o unseals shared.common.sanity, leaving iapi.sanity sealed

          The regression tests ran cleanly (Java 6, Solaris 11 Express).
          Patch ready for review.

          Show
          Kristian Waagan added a comment - I unsealed the wrong package in patch 2b, so here comes patch 3a... Patch 3a does the following: o replaces an import of iapi.sanity.SanityManager in a client class o changes an import to common.shared.sanity.SanityManager in the engine code. This breaks the current pattern, but makes it possible to unseal only the shared.common.sanity package. The reason is that the class JVMInfo is included in both the engine and the client code. o unseals shared.common.sanity, leaving iapi.sanity sealed The regression tests ran cleanly (Java 6, Solaris 11 Express). Patch ready for review.
          Hide
          Kristian Waagan added a comment -

          Committed patch 3a to trunk with revision 1133752.

          I'd like to backport this to 10.8, but not further. Will wait for a while to see if things work out as expected.

          Show
          Kristian Waagan added a comment - Committed patch 3a to trunk with revision 1133752. I'd like to backport this to 10.8, but not further. Will wait for a while to see if things work out as expected.
          Hide
          Kristian Waagan added a comment -

          Backported to 10.8 with revision 1136020.

          Show
          Kristian Waagan added a comment - Backported to 10.8 with revision 1136020.
          Hide
          Kathey Marsden added a comment -

          Reopen for 10.5 backport consideration. If working on the backport for this issue. Temporarily assign yourself and add a comment that you are working on it. When finished, reresolve with the new fix versions or label backport_reject_10_x as appropriate.

          Show
          Kathey Marsden added a comment - Reopen for 10.5 backport consideration. If working on the backport for this issue. Temporarily assign yourself and add a comment that you are working on it. When finished, reresolve with the new fix versions or label backport_reject_10_x as appropriate.
          Hide
          Kathey Marsden added a comment -

          Changing assignee from Kristian to myself for backport

          Show
          Kathey Marsden added a comment - Changing assignee from Kristian to myself for backport
          Hide
          Kathey Marsden added a comment -

          Resolving back to 10.5 and reassigning to Kristian. Neglected to do so back in March.
          10.7: r1304465 | kmarsden | 2012-03-23 09:26:32 -0700 (Fri, 23 Mar 2012) | 5 lines
          10.6: r1304470 | kmarsden | 2012-03-23 09:41:26 -0700 (Fri, 23 Mar 2012) | 6 lines
          10.5: r1304478 | kmarsden | 2012-03-23 09:55:04 -0700 (Fri, 23 , 23 Mar 2012) | 6 lines

          Show
          Kathey Marsden added a comment - Resolving back to 10.5 and reassigning to Kristian. Neglected to do so back in March. 10.7: r1304465 | kmarsden | 2012-03-23 09:26:32 -0700 (Fri, 23 Mar 2012) | 5 lines 10.6: r1304470 | kmarsden | 2012-03-23 09:41:26 -0700 (Fri, 23 Mar 2012) | 6 lines 10.5: r1304478 | kmarsden | 2012-03-23 09:55:04 -0700 (Fri, 23 , 23 Mar 2012) | 6 lines
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update: close all resolved issues that haven't had any activity the last year]

          Show
          Knut Anders Hatlen added a comment - [bulk update: close all resolved issues that haven't had any activity the last year]

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development