db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Bouschen <mbo.t...@spree.de>
Subject Re: status update: ri11 enhancer support for jdk 1.5
Date Thu, 29 Sep 2005 14:27:55 GMT
Hi Martin,

thanks for the comments!

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.

Regards Michael

>
> Hi Michael,
> thanks for the patch, only a few cosmetic comments inline, below.
> I'll send out a summary of our discussion on the jdk 1.5 class
> registration problem and the chosen approach later.
> Martin
>
> 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.
>>
>> Regards Michael
>>
>>>
>>> Michael,
>>> attached is a stable subset of my ri11 enhancer
>>> changes that produce the problem with Test_Query.
>>> Unzip in trunk and run 'maven build' from ri11.
>>> Talk to you next week,
>>> Martin
>>
>>
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/runtime/Bundle.properties 
>>
>> ===================================================================
>> --- 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/runtime/Bundle.properties    
>> (Revision 292403)
>> +++ 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/runtime/Bundle.properties    
>> (Arbeitskopie)
>> @@ -62,12 +62,3 @@
>>  #NOI18N
>>  ERR_CannotSetJDOModel=Cannot set JDOModel for JavaModel instance.
>>  
>> -#
>> -# RuntimeJavaType
>> -#
>> -
>> -# {0} - error location (class.method)
>> -# {1} - implementation method name
>> -# {2} - field name
>> -#NOI18N
>> -ERR_MultipleJavaField={0}: multiple JavaField ''{1}'' for class '{2}''.
>> Index: 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/reflection/ReflectionJavaModel.java

>>
>> ===================================================================
>> --- 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/reflection/ReflectionJavaModel.java
   
>> (Revision 292403)
>> +++ 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/reflection/ReflectionJavaModel.java
   
>> (Arbeitskopie)
>> @@ -87,8 +87,8 @@
>>                      // loaded, Class.forName will load the class which
>>                      // calls RegisterClassListener.registerClass.
>>                      // This will create a new JavaType entry in the 
>> cache.
>> -                    javaType = getJavaType(Class.forName(name, 
>> initialize, -                                                         
>> classLoader));
>> +                    javaType = getJavaTypeInternal(
>> +                        Class.forName(name, initialize, classLoader));
>>                  }
>>                  catch (ClassNotFoundException ex) {
>>                      // cannot find class => return null
>> @@ -121,15 +121,20 @@
>>       */
>>      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());
>
>>              }
>> -            return javaType;
>> +            catch (ClassNotFoundException ex) {
>> +                // ignore
>
>
> I'd add:
>                // ignore, since class has already been loaded
>
>> +            }
>>          }
>> +
>> +        return getJavaTypeInternal(clazz);
>>      }
>>  
>>      /**
>> @@ -170,6 +175,28 @@
>>          return classLoader;
>>      }
>>  
>> +    /**
>> +     * The method returns the JavaType instance for the type name of 
>> the
>> +     * specified class object. It first checks the cache and if 
>> there is no
>> +     * entry for the type name in the cache then it creates a new 
>> JavaType
>> +     * instance for the specified Class object.
>> +     * @param clazz the Class instance representing the type
>> +     * @return a JavaType instance for the name of the specified class
>> +     * object or <code>null</code> if not present in this model 
>> instance.
>> +     */
>> +    protected JavaType getJavaTypeInternal(Class clazz)
>> +    {
>> +        String name = clazz.getName();
>> +        synchronized (types) {
>> +            JavaType javaType = (JavaType)types.get(name);
>> +            if (javaType == null) {
>> +                javaType = createJavaType(clazz);
>> +                types.put(name, javaType);
>> +            }
>> +            return javaType;
>> +        }
>> +    }
>> +
>>      /**       * Creates a new JavaType instance for the specified 
>> Class object.
>>       * This method provides a hook such that ReflectionJavaModel 
>> subclasses
>> Index: 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/Bundle.properties
>> ===================================================================
>> --- 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/Bundle.properties    
>> (Revision 292403)
>> +++ 
>> trunk/ri11/src/java/org/apache/jdo/impl/model/java/Bundle.properties    
>> (Arbeitskopie)
>> @@ -27,7 +27,7 @@
>>  EXC_ClassLoadingError=Error during loading of class ''{0}'': {1}.
>>  
>>  #
>> -# ReflectionJavaType
>> +# BaseReflectionJavaType
>>  #
>>  
>>  # {0} - error location (class.method)
>> @@ -35,6 +35,16 @@
>>  ERR_InvalidNullClassInstance={0}: specified Class instance is null.
>>  
>>  #
>> +# ReflectionJavaType
>> +#
>> +
>> +# {0} - error location (class.method)
>> +# {1} - implementation method name
>> +# {2} - field name
>> +#NOI18N
>> +ERR_MultipleJavaField={0}: multiple JavaField ''{1}'' for class 
>> ''{2}''.
>> +
>> +#
>>  # BaseReflectionJavaField
>>  #
>>  # {0} - class name
>


-- 
Michael Bouschen		Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de	http://www.tech.spree.de/
Tel.:++49/30/235 520-33		Buelowstr. 66			
Fax.:++49/30/2175 2012		D-10783 Berlin			


Mime
View raw message