commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Trimble (JIRA)" <>
Subject [jira] Commented: (JXPATH-127) Change dynamic class loading to consult context class loader.
Date Wed, 25 Feb 2009 07:11:01 GMT


John Trimble commented on JXPATH-127:

Thank you for taking the time to respond to my post. I understand and appreciate your concern
regarding the first patch I attached, and I apologize for the trouble. I looked over Roubtsov's
code and the only thing taken from him code-wise is the use of a dummy SecurityManager to
get the call stack. This isn't necessary so I removed it and uploaded a new patch. Of course,
the patch still uses the same approach to class loading as described in the article (which
may be the root of your IP concern). If you still feel there are IP issues, I can contact
Roubtsov about an IP grant. However, I've included 2 additional patches, based on your suggestion
of using [lang]'s code, one of which might work better anyways.

The patch jxpath-14-classloading-lang-port.patch is, for the most part, a verbatim port of
[lang]'s class loading code. I made some modifications to remove dependence on StringUtil.
It also includes a test case. My concern with using [lang]'s class loading code is that, while
it solves the original problem, it may cause backwards compatibility issues. Specifically,
anyone currently using [jxpath] in an application where the context class loader is set will
now have jxpath attempting to load their classes through the context class loader (which may
not have access to them) instead of the class loader that loaded jxpath as it used to (the
jxpath-14-classloading-widest-scope.patch patch solves this problem by choosing the class
loader with the widest scope).

The patch jxpath-14-classloading-lang-port-with-fallback.patch is also a port of [lang]'s
code but modified to use the current class loader (the class loader that loaded ClassLoaderUtil)
if the context class loader throws a ClassNotFoundException. This should make the class loading
change to [jxpath] transparent to those already using it, and essentially achieves the same
goal as the jxpath-14-classloading-widest-scope.patch patch. So either patch jxpath-14-classloading-widest-scope.patch
or jxpath-14-classloading-lang-port-with-fallback.patch would be best, in my opinion, but
any of them should solve the problem. I have not marked any of them for inclusion due to the
IP concerns with the first and the other two being ports of code, not written by me, from
[lang]. I can change that, but I wanted to err on the side of caution this time. Let me know
what you think and thanks again.

> Change dynamic class loading to consult context class loader.
> -------------------------------------------------------------
>                 Key: JXPATH-127
>                 URL:
>             Project: Commons JXPath
>          Issue Type: Improvement
>            Reporter: John Trimble
>             Fix For: post-1.3
>         Attachments: classloading_fix_jxpath14.patch, jxpath-14-classloading-lang-port-with-fallback.patch,
jxpath-14-classloading-lang-port.patch, jxpath-14-classloading-widest-scope.patch
> For dynamically loading classes, JXPath currently uses Class.forName(...). This means
all classes loaded by JXPath must be visible to whatever class loader loaded JXPath. This
is restrictive in large frameworks and web applications where multiple class loaders are in
use. In particular, in an EJB3 application deployed on Weblogic, the shared jars between different
EJB modules are loaded with a separate class loader than the modules themselves. Consequently,
if the jxpath.jar is among the general libraries for the application, then a specific EJB
module will be unable to reference static methods of classes within its own module in an xpath
expression. For example, consider the following lines of code in an EJB module class:
> JXPathContext context = JXPathContext.newContext(new Object());
> Object value = context.getValue("SomeClass.getSomething()");
> This will fail, with a ClassNotFoundException, if SomeClass is a class in the module,
but jxpath is a general library for the application.
> Attached (or will be once I figure out how) is a patch for JXPath that gets around this
issue. The patch changes the class loading strategy to use the context class loader, when
it appears appropriate, to dynamically load classes. The strategy is based on suggestions
and example code from:
> Assuming all class loaders involved delegate to their parent class loader (as recommended
in the API), currently working code, that uses JXPath, should only be affected by this change
if the current class loader (the class loader that loaded JXPath) is not in an ancestor/descendant
relationship with the context class loader.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message