#2 PATCH: Fix classloader delegation

closed-accepted
nobody
None
5
2007-12-29
2007-12-03
James Berry
No

Rick (filing this patch, repeating from my mail to you earlier today):

Okay, I think I've got it. We simply need to ensure that those classes embedded within winstone (javax.servlet) that are also used by webapps, are loaded from winstone, rather than from the webapp. To do this, we use the following delegation rules:

(1) If the class is in the cache, use that of course.

(2) Invoke the system class loader. This takes care of one aspect of the servlet spec, which specifies that a webapp may not override any java system classes. And since the system class loader will load from winstone as well, it ensures that any classes embedded in winstone (such as javax.servlet) are loaded in preference to classes of the same version within the webapp.

(3) Look for the class within the webapp

(4) Finally, delegate to the webapp's parent class loader; this is either the system class loader or the common class loader. Since we know we've already invoked the system class loader, this step will pick up only any classes that reside in the common class directory.

This works for me, and seems to mostly follow the servlet spec, with the exception that classes from winstone are visible to the webapp. To change that would force winstone to use a bootstrapping jar scheme like tomcat uses, which would imply a refactoring of winstone. Besides, even tomcat's bootstrap scheme violates this prohibition.

In summary: according to my testing, this (a) works, and (b) gets you closer to spec compliance.

Discussion

  • James Berry
    James Berry
    2007-12-03

    WebAppClassLoader.java diff

     
  • James Berry
    James Berry
    2007-12-03

    WebAppClassLoader.java

     
  • James Berry
    James Berry
    2007-12-03

    Logged In: YES
    user_id=1231469
    Originator: YES

    Adding entire changed WebAppClassLoader.java file (this should be the result of applying the patch attached earlier).
    File Added: WebappClassLoader.java

     
  • Rick Knowles
    Rick Knowles
    2007-12-29

    Logged In: YES
    user_id=716353
    Originator: NO

    This has been incorporated into the main tree now. Thanks.

    To all: Please report any observed problems with classloading, as this is fairly low-level change with difficult to test side effects.

     
  • Rick Knowles
    Rick Knowles
    2007-12-29

    • status: open --> closed-accepted