If this change were to be implemented then it should be implemented the same way as the delegate flag since that controls a similar behaviour. I'd lean towards to boolean flag that simply disabled the code block (0.2) that checked the system class loader.

Note that the system class loader is checked first to enforce the specification requirement that web applications must not be allowed to override Java SE platform classes. I'd hesitate before adding an option to disable this check because of the specification requirement.

I wonder if there isn't a better solution to this issue. The system class loader might not be the best class loader to use here. It is really the bootstrap class loader that is required but you can't get a reference to that in some JREs - including Oracle's. The class loader hierarchy for an Oracle JVM is system->ext->bootstrap so using the ext class loader would work in that case.

I'm thinking that rather than using the system class loader in this case the parent of the system class loader should be used (if it has one). That should both fix this issue and still enforce the specification requirement for not allowing the overriding of Java SE platformclasses.

Switching to use a different classloader does sound a lot easier. Should I use the parent of the system classloader? It sounds like we really want the  bootstrap loader. Couldn't I recursively get the parent loaders, until I run out, to get the bootstrap loader?

Doing that won't get you the bootstrap class loader in an Oracle JRE - you'll get the ext loader (which is good enough for this use case).

For the sake of clarity, my suggestion is start at the system class loader, recursively get the parent and use the last non-null value you find.

*** This bug has been marked as a duplicate of bug 55945 ***

Sorry - wrong bug.

I've fixed this in 8.0.x for 8.0.0 and in 7.0.x for 7.0.51 onwards.

As I was cleaning up the use of the system class loader I also refactored the handling of parent==null which fell back to the system class loader to make it a little (probably not noticeably) faster.

You beat me to it.  Thanks for all of your help.

Hi

this totaally breaks tomcat and tomee usage in embeded mode (+ has a lot of side effect in normal mode).

1) the j2seClassLoader is not overridable in children classloader which is a pain since system was
2) system is no more used
3) it breaks compatibility

can you revert it and make it active just with a flag is asked?

Side note: for a minor this is an important change which should have been showed a bit more (7.1 maybe)

This change breaks previous behaviour. We have an app that sets some static variable values and then launches embedded tomcat. Embeded tomcat has a war file that inspects values of these static variables. This no longer seems to work.

We want to use a newer version of tomcat in order to close any security issues. Is there is any way to add a flag to be able to revert to the old behaviour?

This configurable. See this thread for details:
http://markmail.org/thread/mid36pgk7nckp2rr

Thank Mark for the link. I have trouble understanding how to configure to follow the old behaviour.

Are you suggesting to override the WebappClassLoader class?

I was able to figure out the fix. When I added the following line to context.xml file, class loader behaviour was restored:
<Loader delegate="true"/>

Thanks

(In reply to Mark Thomas from comment #6)

The essential bit of r1559153 / r1559134 is the following change:

@@ -1186,9 +1200,9 @@ public class WebappClassLoader extends U
         // (0.2) Try loading the class with the system class loader, to prevent
         //       the webapp from overriding J2SE classes
         String resourceName = binaryNameToPath(name, false);
-        if (system.getResource(resourceName) != null) {
+        if (j2seClassLoader.getResource(resourceName) != null) {
             try {
-                clazz = system.loadClass(name);
+                clazz = j2seClassLoader.loadClass(name);

The old code used 'System' classloader - the JVM CLASSPATH.
The new code uses 'Bootstrap' classloader - the topmost non-null parent of System class loader - the one that provides Java SE core classes.

As such, class-loader-howto,html has to be corrected.
The classes lookup order in 7.0.50 and earlier is:

 *  Bootstrap classes of your JVM
 *  System class loader classes (described above)
 *  /WEB-INF/classes of your web application
 *  /WEB-INF/lib/*.jar of your web application
 *  Common class loader classes (described above)

For 8.0.0 and 7.0.52 and later it now is

 *  Bootstrap classes of your JVM
 *  /WEB-INF/classes of your web application
 *  /WEB-INF/lib/*.jar of your web application
 *  System class loader classes (described above)
 *  Common class loader classes (described above)

I am REOPENING this issue to apply this documentation fix. It is worth noting this in migration guide.

It may be worth to add that if one configures <Loader delegate="true"/>,
the above order becomes

 *  Bootstrap classes of your JVM
 *  System class loader classes (described above)
 *  Common class loader classes (described above)
 *  /WEB-INF/classes of your web application
 *  /WEB-INF/lib/*.jar of your web application


One use case when jar is added by Java to the system classloader is using -javaagent option.
Documentation:
http://docs.oracle.com/javase/7/docs/api/java/lang/instrument/package-summary.html#package_description

A thread:
http://tomcat.markmail.org/thread/trd7yj46qajqra2v
Of course, such jar files should not be in WEB-INF/lib directory.

Docs updated, note added to the 7.0.x changelog and th 7.0.x upgrade guide modified to add a section on notable changes with this as the first.