uima-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marshall Schor (JIRA)" <...@uima.apache.org>
Subject [jira] [Updated] (UIMA-5038) UV3 refactor class loading for consistency / maintainability
Date Thu, 04 Aug 2016 16:06:20 GMT

     [ https://issues.apache.org/jira/browse/UIMA-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Marshall Schor updated UIMA-5038:
---------------------------------
    Description: 
There are 4 main places the UIMA framework loads classes: the Framework itself at startup,
Resources, Annotators (a kind of resource) and JCas Classes.  The last 3 can make use of UIMA
extension class loaders, or PEAR isolation class loaders.

Refactor the code to have these where possible use common code.

One issue to confirm: the current code uses 2 methods for loading classes:
Class.forName and classloader.loadClass.  The Class.forName has a variant which takes 3 arguments,
making it functionally equivalent to the loadClass.  But its operation is subtly different
in that it caches the reference to the loaded class, *even if it isn't the defining class
loader*, whereas loadClass doesn't do this.  See http://blog.bjhargrave.com/2007/09/classforname-caches-defined-class-in.html
for a discussion of this, and the comments for a reply from Oracle tech support saying:

bq. Applications should basically never call Classloader.loadClass(). It may appear to work
but it is often subtly wrong, can be a source of latent bugs, and is almost never the best
choice. They should instead call Class.forName() using the 3 parameter version that takes
a specific Classloader instance.

Our current code always (I think) uses Class.forName for the case where there is no UIMA class
loader, and loadClass otherwise.  I think this would cause failures in an OSGi loading scenario,
where one of the parent loaders was an OSGi loader and it dynamically switched its "parent".
 The expectation would be that the child loader (the UIMA class loader), the next time it
was called to load something *not* in its set of classes, would delegate to the parent chain
and the OSGi loader would now load from whatever was it's current "parent".  But instead,
because the UIMA loader would have "cached" the class, it would instead be stuck getting the
old class forever (unless the old class was eventually GC'd)  

Therefore, I think to work properly in this scenario, we must always use loadClass, and never
Class.forName.  

The one known issue of loading named arrays doesn't arise in our case, unless from some highly
usual user code, and we can catch this and use Class.forName in this instance.

  was:
There are 4 main places the UIMA framework loads classes: the Framework itself at startup,
Resources, Annotators (a kind of resource) and JCas Classes.  The last 3 can make use of UIMA
extension class loaders, or PEAR isolation class loaders.

Refactor the code to have these where possible use common code.

One issue to confirm: the current code uses 2 methods for loading classes:
Class.forName and classloader.loadClass.  The Class.forName has a variant which takes 3 arguments,
making it functionally equivalent to the loadClass.  But its operation is subtly different
in that it caches the reference to the loaded class, *even if it isn't the defining class
loader*, whereas loadClass doesn't do this.  See http://blog.bjhargrave.com/2007/09/classforname-caches-defined-class-in.html
for a discussion of this, and the comments for a reply from Oracle tech support saying:

bq. Applications should basically never call Classloader.loadClass(). It may appear to work
but it is often subtly wrong, can be a source of latent bugs, and is almost never the best
choice. They should instead call Class.forName() using the 3 parameter version that takes
a specific Classloader instance.

Our current code always (I think) uses Class.forName for the case where there is no UIMA class
loader, and loadClass otherwise.  I think this is probably the best choice, as it may enable
correct operation in the presence of applications using OSGi or Thread Local Class Loader
conventions,  so I'm inclined to leave it (but this Jira will still do the refactoring). 


> UV3 refactor class loading for consistency / maintainability
> ------------------------------------------------------------
>
>                 Key: UIMA-5038
>                 URL: https://issues.apache.org/jira/browse/UIMA-5038
>             Project: UIMA
>          Issue Type: Improvement
>          Components: Core Java Framework
>            Reporter: Marshall Schor
>            Priority: Trivial
>             Fix For: 3.0.0SDKexp
>
>
> There are 4 main places the UIMA framework loads classes: the Framework itself at startup,
Resources, Annotators (a kind of resource) and JCas Classes.  The last 3 can make use of UIMA
extension class loaders, or PEAR isolation class loaders.
> Refactor the code to have these where possible use common code.
> One issue to confirm: the current code uses 2 methods for loading classes:
> Class.forName and classloader.loadClass.  The Class.forName has a variant which takes
3 arguments, making it functionally equivalent to the loadClass.  But its operation is subtly
different in that it caches the reference to the loaded class, *even if it isn't the defining
class loader*, whereas loadClass doesn't do this.  See http://blog.bjhargrave.com/2007/09/classforname-caches-defined-class-in.html
for a discussion of this, and the comments for a reply from Oracle tech support saying:
> bq. Applications should basically never call Classloader.loadClass(). It may appear to
work but it is often subtly wrong, can be a source of latent bugs, and is almost never the
best choice. They should instead call Class.forName() using the 3 parameter version that takes
a specific Classloader instance.
> Our current code always (I think) uses Class.forName for the case where there is no UIMA
class loader, and loadClass otherwise.  I think this would cause failures in an OSGi loading
scenario, where one of the parent loaders was an OSGi loader and it dynamically switched its
"parent".  The expectation would be that the child loader (the UIMA class loader), the next
time it was called to load something *not* in its set of classes, would delegate to the parent
chain and the OSGi loader would now load from whatever was it's current "parent".  But instead,
because the UIMA loader would have "cached" the class, it would instead be stuck getting the
old class forever (unless the old class was eventually GC'd)  
> Therefore, I think to work properly in this scenario, we must always use loadClass, and
never Class.forName.  
> The one known issue of loading named arrays doesn't arise in our case, unless from some
highly usual user code, and we can catch this and use Class.forName in this instance.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message