Hi Martin,
thanks for the info! Maybe it makes sense to discuss this over the
phone. I have a meeting from 9-10am PDT tomorrow, so we could talk
before 9am (which is a little early for you) or on Friday.
Regards Michael
Hi Michael,
unfortunately, the fix for the class registration problem, as we
discussed it and as it was checked in, causes most of the ri11
security manager junit tests to fail (138 out of 149 tests).
The symptom is an AccessControlException ("getClassLoader") within
method Class.forName(), called from ReflectionJavaModel.java:130.
The issue is exposed by an almost trivial change that we discussed
(see below): When calling Class.forName() to initialize a class, we
decided to use the class' classloader instead of the one stored in
the JavaModel instance. As you pointed out, we then must fetch the
classloader in a doPrivileged block. (This block succeeds, the
exception is really raised within Class.forName()).
Though the code assumes that the stored and the fetched classloader
are always the same here, we discussed that if they're not, this
would result in an internal error that is very hard to track down,
since the consequences might show up later and elsewhere depending
on the order in which classes are registered. We considered to
guard against this case with an assertion or an explicit check.
It turns out the stored and the fetched classloader may differ for
two reasons, I think.
First, clazz.getClassLoader() returns null for a number of classes
like Date and ArrayList. Craig has pointed out that this is the
specified behaviour for classes loaded by the bootstrap classloader.
The javadoc on Class.forName() says that if the loader argument is
null (and other conditions apply), the security manager is checked
for permission ("getClassLoader") to access the bootstrap class
loader. This check fails.
When using the stored classloader, which is always non-null, instead
of the fetched one as argument to clazz.forName(), this method does
not issue a "getClassLoader" request, and all tests security manager
junit tests pass then.
But I'm not sure this is the right solution either. Essentially,
we'd represent Date, ArrayList etc. a multiple times in the Java
model - in each model instance per application classloader - while
they're only represented once in the JVM, by the bootstrap loader.
There might be another case to consider why the stored and the
fetched classloader may differ: An application that uses multiple
classloaders forming a parent-child hierarchy, it may happen that
a PC class is loaded by a child classloader while it's superclasses
or the persistent field types are loaded by a parent classloader.
In this case, we probably do not want to have the type universe be
duplicated in each java model instance (i.e. per classloader), but
rather represent a class in the java model only as many times as
it's loaded in the VM, that is, per "owning" classloader.
Now, I do not know if we're already doing this (class represented in
java model only for "owning" classloader), but it seems to me that
the implicit assumption in ReflectionJavaModel.getJavaType() that the
stored and the fetched classloader are always the same does not hold.
That's why I'm not sure that always using the stored classloader
(instead of the fetched one) would be a general solution either.
Any thoughts?
Sorry for the long email. If you'd rather like to discuss details
over phone, I'd have some time on Thursday morning (until 9:30am) or
on Friday during or after our JDO phone con. Craig and I discussed
this issue briefly today.
Thx,
Martin
Michael Bouschen wrote:
About the issue of a class argument of a wrong classloader passed to
the JavaModel method getJavaType: I agree the code should use the
class loader from the class object. The only issue is that the call
clazz.getClassLoader() might result in a SecurityException, so I need
to put this call into a doPrivileged block. I already have a
convenience method in RuntimeJavaModelFactory, I just need to move
this method to a class that is accessible in both places.
Michael Bouschen wrote:
Hi Martin,
attached you find a patch for the JDOModel implementation. It
initializes a class instance if the model instance runs with the
initialize=true flag. It also fixes the MissingResourceException. I
could successfully run the ri tests in a workspace including your
enhancer plus my JDOModel changes.
I would port the JDOModel changes to core20 and check them in, if
you agree with the changes.
trunk/ri11/src/java/org/apache/jdo/impl/model/java/reflection/ReflectionJavaModel.java
public JavaType getJavaType(Class clazz)
{
- String name = clazz.getName();
- synchronized (types) {
- JavaType javaType = (JavaType)types.get(name);
- if (javaType == null) {
- javaType = createJavaType(clazz);
- types.put(name, javaType);
+ if (clazz == null)
+ return null;
+ + if (initialize) {
+ try {
+ // make sure the class is initialized
+ Class.forName(clazz.getName(), initialize,
classLoader);
Since this is a public method, there's a chance for a bug that we're
called here with a class argument of a wrong classloader, I think.
If that happened, we'd load and initialize the class in the wrong
place. So, I'd change this line, making an assumption explicit, to:
Class.forName(clazz.getName(), initialize,
clazz.getClassLoader());
Or, if we allowed for assertions, we could keep the line but just add:
assert (classLoader == clazz.getClassLoader());
}
--
Michael Bouschen [EMAIL PROTECTED] Engineering GmbH
mailto:[EMAIL PROTECTED] http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin
