db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d..@apache.org
Subject svn commit: r1620391 - in /db/derby/code/branches/10.11: ./ java/engine/org/apache/derby/impl/services/timer/ java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/ java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/ java...
Date Mon, 25 Aug 2014 17:53:32 GMT
Author: dag
Date: Mon Aug 25 17:53:31 2014
New Revision: 1620391

URL: http://svn.apache.org/r1620391
Log:
DERBY-6619 After silently swallowing SecurityExceptions, Derby can leak class loaders

Backport from trunk svn 1620378.

Patch derby-6619-2.

The fix introduced in DERBY-3745 correctly is there in order to
protect against the case where the thread that starts Derby, has a
context class loader that is different from the system class
loader. In such cases, if the timer thread inherits the context class
loader, the context class loader will stay in memory until the Derby
engine is shut down, even if all other references to the class loader
are gone.

If the context class loader is the same as the system class loader, on
the other hand, such a "leak" would not be a problem, since the system
class loader will stay in memory until the JVM is shut down anyway.
We take advantage of this and only attempt to change the context class
loader if it is different from the system class loader. With this
patch, no warning is printed to derby.log when starting the server
from the command line, and there's no warning when starting the server
using the API with a security manager installed when the context class
loader hasn't been changed from the default. However, if the server is
started using the API with a non-default context class loader, we do
see warnings in derby.log if a security manager is installed and the
permission to set the class loader is missing.

Added tests for this behavior. Moved utility methods from
UpgradeClassLoader to ClassLoaderTestSetup, a new decorator. It seemed
more logical to put them there to allow reuse.


Added:
    db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/SecureServerTest.policy
      - copied unchanged from r1620379, db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/SecureServerTest.policy
    db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/junit/ClassLoaderTestSetup.java
      - copied unchanged from r1620379, db/derby/code/trunk/java/testing/org/apache/derbyTesting/junit/ClassLoaderTestSetup.java
Modified:
    db/derby/code/branches/10.11/   (props changed)
    db/derby/code/branches/10.11/java/engine/org/apache/derby/impl/services/timer/SingletonTimerFactory.java
    db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/SecureServerTest.java
    db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/PhaseChanger.java
    db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeClassLoader.java
    db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeTrajectoryTest.java
    db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/junit/BaseTestSetup.java

Propchange: db/derby/code/branches/10.11/
------------------------------------------------------------------------------
  Merged /db/derby/code/trunk:r1620379

Modified: db/derby/code/branches/10.11/java/engine/org/apache/derby/impl/services/timer/SingletonTimerFactory.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.11/java/engine/org/apache/derby/impl/services/timer/SingletonTimerFactory.java?rev=1620391&r1=1620390&r2=1620391&view=diff
==============================================================================
--- db/derby/code/branches/10.11/java/engine/org/apache/derby/impl/services/timer/SingletonTimerFactory.java
(original)
+++ db/derby/code/branches/10.11/java/engine/org/apache/derby/impl/services/timer/SingletonTimerFactory.java
Mon Aug 25 17:53:31 2014
@@ -159,13 +159,35 @@ public class SingletonTimerFactory
 
     // Helper methods
 
+    /**
+     * Get the context class loader if it's different from the system
+     * class loader.
+     *
+     * @return the context class loader of the current thread if it is
+     *   different from the system class loader and we have permission
+     *   to read the class loader, or {@code null} otherwise
+     */
     private ClassLoader getContextClassLoader() {
         try {
             return AccessController.doPrivileged(
                     new PrivilegedAction<ClassLoader>() {
                 @Override
                 public ClassLoader run() {
-                    return Thread.currentThread().getContextClassLoader();
+                    ClassLoader cl =
+                        Thread.currentThread().getContextClassLoader();
+                    if (cl == ClassLoader.getSystemClassLoader()) {
+                        // If the context class loader is the same as the
+                        // system class loader, we are not worried that the
+                        // timer thread will lead a class loader. (The
+                        // system class loader will stay in memory for the
+                        // lifetime of the JVM anyway, so it's not a problem
+                        // that the timer thread keeps a reference to it.)
+                        // Return null to signal that the context class loader
+                        // doesn't need to be changed.
+                        return null;
+                    } else {
+                        return cl;
+                    }
                 }
             });
         } catch (SecurityException se) {

Modified: db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/SecureServerTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/SecureServerTest.java?rev=1620391&r1=1620390&r2=1620391&view=diff
==============================================================================
--- db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/SecureServerTest.java
(original)
+++ db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/SecureServerTest.java
Mon Aug 25 17:53:31 2014
@@ -21,19 +21,18 @@
 
 package org.apache.derbyTesting.functionTests.tests.derbynet;
 
-import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.util.ArrayList;
 import java.util.Arrays;
 import junit.framework.Test;
+import org.apache.derby.drda.NetworkServerControl;
 import org.apache.derbyTesting.functionTests.util.PrivilegedFileOpsForTests;
 import org.apache.derbyTesting.junit.BaseJDBCTestCase;
 import org.apache.derbyTesting.junit.BaseTestSuite;
+import org.apache.derbyTesting.junit.ClassLoaderTestSetup;
 import org.apache.derbyTesting.junit.Derby;
 import org.apache.derbyTesting.junit.NetworkServerTestSetup;
 import org.apache.derbyTesting.junit.SecurityManagerSetup;
@@ -142,6 +141,10 @@ public class SecureServerTest extends Ba
          _outcome = outcome;
     }
 
+    public SecureServerTest(String fixture) {
+        super(fixture);
+    }
+
     ///////////////////////////////////////////////////////////////////////////////////
     //
     // JUnit MACHINERY
@@ -181,9 +184,7 @@ public class SecureServerTest extends Ba
         // this wildcard port is rejected by the server right now
         //suite.addTest( decorateTest( false,  true, null, IPV6W, RUNNING_SECURITY_BOOTED
) );
         
-        suite.addTest( decorateTest( true,  false, null, null, RUNNING_SECURITY_NOT_BOOTED
) );
-        suite.addTest( decorateTest( true,  true, null, null, RUNNING_SECURITY_NOT_BOOTED
) );
-        
+        suite.addTest( makeDerby6619Test() );
         return suite;
     }
 
@@ -306,12 +307,33 @@ public class SecureServerTest extends Ba
         return list.toArray(new String[list.size()]);
     }
     
+    // Policy which lacks the permission to set the context class loader.
+    final static String POLICY6619 =
+            "org/apache/derbyTesting/functionTests/" +
+            "tests/derbynet/SecureServerTest.policy";
+
+    private static Test makeDerby6619Test() {
+        Test t = new SecureServerTest("test6619");
+        t = TestConfiguration.clientServerDecorator(t);
+        t = new SecurityManagerSetup(t, POLICY6619);
+        t = new ClassLoaderTestSetup(t);
+        return t;
+    }
+
     ///////////////////////////////////////////////////////////////////////////////////
     //
     // JUnit TESTS
     //
     ///////////////////////////////////////////////////////////////////////////////////
     
+    public void test6619() throws Exception {
+        NetworkServerControl nsc =
+                NetworkServerTestSetup.getNetworkServerControl();
+        NetworkServerTestSetup.waitForServerStart(nsc);
+         // non standard class loader, so expect to see the warning on derby.log
+        assertWarningDerby6619("derby.system.home", true);
+    }
+
     /**
      * Verify if the server came up and if so, was a security manager installed.
      */
@@ -325,9 +347,7 @@ public class SecureServerTest extends Ba
 
         assertEquals( myName + ": serverCameUp = " + serverCameUp, _outcome.serverShouldComeUp(),
serverCameUp );
 
-        if (!_unsecureSet) {
-            assertWarningDerby6619();
-        }
+        assertWarningDerby6619("user.dir", false); // standard class loader
 
         if (!(runsWithEmma() || runsWithJaCoCo())) {
             // With Emma we run without the security manager, so we can't
@@ -507,11 +527,20 @@ public class SecureServerTest extends Ba
                  "security exception:",
              "This may lead to class loader leak"};
 
-    private void assertWarningDerby6619() throws IOException {
+
+    private void assertWarningDerby6619(String logLocation, boolean expected)
+            throws IOException {
+
         final String logFileName =
-                getSystemProperty("user.dir") + File.separator + "derby.log";
-        if (!DerbyNetAutoStartTest.checkLog(logFileName, expected6619)) {
-            fail("Expected warning on derby.log cf DERBY-6619");
+                getSystemProperty(logLocation) + File.separator + "derby.log";
+        if (DerbyNetAutoStartTest.checkLog(logFileName, expected6619)) {
+            if (!expected) {
+                fail("Expected no warning on derby.log cf DERBY-6619");
+            }
+        } else {
+            if (expected) {
+                fail("Expected warning on derby.log cf DERBY-6619");
+            }
         }
     }
 }

Modified: db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/PhaseChanger.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/PhaseChanger.java?rev=1620391&r1=1620390&r2=1620391&view=diff
==============================================================================
--- db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/PhaseChanger.java
(original)
+++ db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/PhaseChanger.java
Mon Aug 25 17:53:31 2014
@@ -31,6 +31,7 @@ import junit.framework.Test;
 
 import org.apache.derbyTesting.junit.BaseTestCase;
 import org.apache.derbyTesting.junit.BaseTestSetup;
+import org.apache.derbyTesting.junit.ClassLoaderTestSetup;
 import org.apache.derbyTesting.junit.JDBC;
 import org.apache.derbyTesting.junit.JDBCDataSource;
 import org.apache.derbyTesting.junit.TestConfiguration;
@@ -87,7 +88,7 @@ final class PhaseChanger extends BaseTes
         
         if (loader != null) {
             previousLoader = Thread.currentThread().getContextClassLoader();
-            UpgradeClassLoader.setThreadLoader(loader);
+            ClassLoaderTestSetup.setThreadLoader(loader);
         }
          
         DataSource ds = JDBCDataSource.getDataSource();
@@ -154,7 +155,7 @@ final class PhaseChanger extends BaseTes
         clearDerby23ThreadLocals(contextService);
 
         if (loader != null)
-            UpgradeClassLoader.setThreadLoader(previousLoader);       
+            ClassLoaderTestSetup.setThreadLoader(previousLoader);
         loader = null;
         previousLoader = null;
         

Modified: db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeClassLoader.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeClassLoader.java?rev=1620391&r1=1620390&r2=1620391&view=diff
==============================================================================
--- db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeClassLoader.java
(original)
+++ db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeClassLoader.java
Mon Aug 25 17:53:31 2014
@@ -92,34 +92,6 @@ public class UpgradeClassLoader
     }
 
     /**
-     * <p>
-     * Force this thread to use a specific class loader.
-     * </p>
-     */
-    public static void setThreadLoader(final ClassLoader which) {
-        AccessController.doPrivileged(new PrivilegedAction<Void>() {
-            public Void run() {
-                java.lang.Thread.currentThread().setContextClassLoader(which);
-              return null;
-            }
-        });
-    }
-    
-    /**
-     * <p>
-     * Retrieve the class loader currently being used by this thread.
-     * </p>
-     */
-    public static ClassLoader getThreadLoader() {
-        return AccessController.doPrivileged(
-                new PrivilegedAction<ClassLoader>() {
-            public ClassLoader run() {
-                return Thread.currentThread().getContextClassLoader();
-            }
-        });
-    }
-
-    /**
      * Get the location of jars of old release. The location is specified 
      * in the property derbyTesting.oldReleasePath. If derbyTesting.oldReleasePath
      * is set to the empty string it is ignored.

Modified: db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeTrajectoryTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeTrajectoryTest.java?rev=1620391&r1=1620390&r2=1620391&view=diff
==============================================================================
--- db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeTrajectoryTest.java
(original)
+++ db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/UpgradeTrajectoryTest.java
Mon Aug 25 17:53:31 2014
@@ -34,6 +34,7 @@ import junit.extensions.TestSetup;
 import junit.framework.Test;
 import org.apache.derbyTesting.junit.BaseJDBCTestCase;
 import org.apache.derbyTesting.junit.BaseTestSuite;
+import org.apache.derbyTesting.junit.ClassLoaderTestSetup;
 import org.apache.derbyTesting.junit.JDBCClient;
 import org.apache.derbyTesting.junit.JDBCClientSetup;
 import org.apache.derbyTesting.junit.JDBCDataSource;
@@ -522,7 +523,7 @@ public class UpgradeTrajectoryTest exten
     private void compareDatabases( Version version, String leftDatabaseName, String rightDatabaseName
)
         throws Exception
     {
-        UpgradeClassLoader.setThreadLoader( version.getClassLoader() );
+        ClassLoaderTestSetup.setThreadLoader( version.getClassLoader() );
 
         DataSource leftDS = makeDataSource( leftDatabaseName );
         DataSource rightDS = makeDataSource( rightDatabaseName );
@@ -811,7 +812,7 @@ public class UpgradeTrajectoryTest exten
     private void createDatabase( Version version, String logicalDatabaseName )
         throws Exception
     {
-        UpgradeClassLoader.setThreadLoader( version.getClassLoader() );
+        ClassLoaderTestSetup.setThreadLoader( version.getClassLoader() );
 
         DataSource ds = bootDatabase( logicalDatabaseName );
 
@@ -830,7 +831,7 @@ public class UpgradeTrajectoryTest exten
     private void upgradeDatabase( Version softwareVersion, Version dataVersion, boolean hardUpgrade,
String logicalDatabaseName )
         throws Exception
     {
-        UpgradeClassLoader.setThreadLoader( softwareVersion.getClassLoader() );
+        ClassLoaderTestSetup.setThreadLoader(softwareVersion.getClassLoader());
 
         DataSource ds = upgradeDatabase( logicalDatabaseName, hardUpgrade );
 
@@ -971,11 +972,14 @@ public class UpgradeTrajectoryTest exten
     private void saveOriginalClassLoader()
     {
         // remember the original class loader so that we can reset
-        if ( _originalClassLoader.get() == null ) { _originalClassLoader.set( UpgradeClassLoader.getThreadLoader()
); }
+        if ( _originalClassLoader.get() == null ) { 
+            _originalClassLoader.set( ClassLoaderTestSetup.getThreadLoader() ); 
+        }
     }
     private void restoreOriginalClassLoader()
     {
-        UpgradeClassLoader.setThreadLoader( (ClassLoader) _originalClassLoader.get() );
+        ClassLoaderTestSetup.setThreadLoader(
+                (ClassLoader) _originalClassLoader.get() );
     }
 
     private String stringifyUpgradeRequests()

Modified: db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/junit/BaseTestSetup.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/junit/BaseTestSetup.java?rev=1620391&r1=1620390&r2=1620391&view=diff
==============================================================================
--- db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/junit/BaseTestSetup.java
(original)
+++ db/derby/code/branches/10.11/java/testing/org/apache/derbyTesting/junit/BaseTestSetup.java
Mon Aug 25 17:53:31 2014
@@ -42,7 +42,8 @@ public abstract class BaseTestSetup exte
      * and then call the part's run method to run the decorator and
      * the test it wraps.
      */
-    public final void run(TestResult result)
+    @Override
+    public void run(TestResult result)
     {
         // install a default security manager if one has not already been
         // installed



Mime
View raw message