[jira] [Commented] (UIMA-5038) UV3 refactor class loading for consistency / maintainability

["Marshall Schor (JIRA)" <de...@uima.apache.org>]

    [ https://issues.apache.org/jira/browse/UIMA-5038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15668628#comment-15668628 ] 

Marshall Schor commented on UIMA-5038:
--------------------------------------

Some refactoring was done to use common code for loading.  I didn't change it to use loadClass - it still uses Class.forName, because of reading there's perhaps some other mysterious edge cases if you don't use the forName style.

If a OSGi user complains, we may revisit.

> 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)