You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/11/05 16:41:54 UTC

svn commit: r1539036 - in /tomcat/trunk: java/org/apache/catalina/loader/WebappLoader.java test/org/apache/catalina/loader/TestVirtualWebappLoader.java test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java

Author: markt
Date: Tue Nov  5 15:41:53 2013
New Revision: 1539036

URL: http://svn.apache.org/r1539036
Log:
Fix remainder of failing tests and a related TODO spotted in the tests as well.

Modified:
    tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java
    tomcat/trunk/test/org/apache/catalina/loader/TestVirtualWebappLoader.java
    tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java

Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java?rev=1539036&r1=1539035&r2=1539036&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java Tue Nov  5 15:41:53 2013
@@ -305,6 +305,9 @@ public class WebappLoader extends Lifecy
 
 
     public String[] getLoaderRepositories() {
+        if (classLoader == null) {
+            return new String[0];
+        }
         URL[] urls = classLoader.getURLs();
         String[] result = new String[urls.length];
         for (int i = 0; i < urls.length; i++) {

Modified: tomcat/trunk/test/org/apache/catalina/loader/TestVirtualWebappLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/loader/TestVirtualWebappLoader.java?rev=1539036&r1=1539035&r2=1539036&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/loader/TestVirtualWebappLoader.java (original)
+++ tomcat/trunk/test/org/apache/catalina/loader/TestVirtualWebappLoader.java Tue Nov  5 15:41:53 2013
@@ -65,9 +65,9 @@ public class TestVirtualWebappLoader ext
         String[] repos = loader.getLoaderRepositories();
         assertEquals(3,repos.length);
         loader.stop();
-        // ToDo: Why doesn't remove repositories?
+
         repos = loader.getLoaderRepositories();
-        assertEquals(3, repos.length);
+        assertEquals(0, repos.length);
 
         // no leak
         loader.start();

Modified: tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java?rev=1539036&r1=1539035&r2=1539036&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java (original)
+++ tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java Tue Nov  5 15:41:53 2013
@@ -251,6 +251,8 @@ public class TestWebappClassLoaderWeavin
         assertEquals("The second result is not correct.", "Hello, Weaver #2!", result);
 
         WebappClassLoader copiedLoader = this.loader.copyWithoutTransformers();
+        // class loader needs to be started to populate URLs
+        copiedLoader.start();
 
         result = invokeDoMethodOnClass(copiedLoader, "TesterNeverWeavedClass");
         assertEquals("The third result is not correct.", "This will never be weaved.", result);



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1539036 - in /tomcat/trunk: java/org/apache/catalina/loader/WebappLoader.java test/org/apache/catalina/loader/TestVirtualWebappLoader.java test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java

Posted by Mark Thomas <ma...@apache.org>.
On 05/11/2013 16:47, Nick Williams wrote:
> copyWithoutTransformers(), as defined in the interface InstrumentableClassLoader, returns a ClassLoader. The start() method is not defined in ClassLoader, it is specific to WebappClassLoader. Furthermore, code calling copyWithoutTransformers() won't have access to WebappClassLoader to call start() reflectively if a SecurityManager is enabled.
> 
> So, the copyWithoutTransformers() method needs to call start() before it returns the copied class loader. Otherwise, it will be useless to JPA providers and the like.

Fixed. Thanks.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1539036 - in /tomcat/trunk: java/org/apache/catalina/loader/WebappLoader.java test/org/apache/catalina/loader/TestVirtualWebappLoader.java test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java

Posted by Nick Williams <ni...@nicholaswilliams.net>.
On Nov 5, 2013, at 9:41 AM, markt@apache.org wrote:

> Author: markt
> Date: Tue Nov  5 15:41:53 2013
> New Revision: 1539036
> 
> URL: http://svn.apache.org/r1539036
> Log:
> Fix remainder of failing tests and a related TODO spotted in the tests as well.
> 
> <snip />
> 
> Modified: tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java?rev=1539036&r1=1539035&r2=1539036&view=diff
> ==============================================================================
> --- tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java (original)
> +++ tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoaderWeaving.java Tue Nov  5 15:41:53 2013
> @@ -251,6 +251,8 @@ public class TestWebappClassLoaderWeavin
>         assertEquals("The second result is not correct.", "Hello, Weaver #2!", result);
> 
>         WebappClassLoader copiedLoader = this.loader.copyWithoutTransformers();
> +        // class loader needs to be started to populate URLs
> +        copiedLoader.start();
> 
>         result = invokeDoMethodOnClass(copiedLoader, "TesterNeverWeavedClass");
>         assertEquals("The third result is not correct.", "This will never be weaved.", result);

copyWithoutTransformers(), as defined in the interface InstrumentableClassLoader, returns a ClassLoader. The start() method is not defined in ClassLoader, it is specific to WebappClassLoader. Furthermore, code calling copyWithoutTransformers() won't have access to WebappClassLoader to call start() reflectively if a SecurityManager is enabled.

So, the copyWithoutTransformers() method needs to call start() before it returns the copied class loader. Otherwise, it will be useless to JPA providers and the like.

Nick
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org