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

Uwe Schindler commented on SOLR-3424:
-------------------------------------

One thing to add:

bq. the cache only caches "resolves" not instantiations of encoders, the JVM 
caches Class.forName() lookups, so why cache it again?

You may argue, that the lookup may be more expensive, as it uses a try..catch 
block (first try in expected commons package, later try a full class name). I 
think the chain of Try...Catch with ClassNoFound should be changed to do it 
more smart: If the name of encoder contains a period (indexOf('.')>=0), look it 
up as class name, otherwise prefix it with the commons package name. This way, 
the JVM cache for loaded classes can be used and the cache is completely 
useless.


I like in your fix, that it also changed the broken uppercasing: It should only 
do that for the builtins, class names itsself are case sensitive.

+1 to remove the cache and only keep the static builtins (and please: as 
Collections.unmodifiableMap!!!), lookup non-builtins without 
try...catch(ClassNotFound). The error message should only list the builtins and 
mention that otherwise the name should be a full class name.
                
> PhoneticFilterFactory threadsafety bug
> --------------------------------------
>
>                 Key: SOLR-3424
>                 URL: https://issues.apache.org/jira/browse/SOLR-3424
>             Project: Solr
>          Issue Type: Bug
>          Components: Schema and Analysis
>    Affects Versions: 3.6, 4.0
>            Reporter: David Smiley
>            Assignee: David Smiley
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: SOLR-3424_PhoneticFilterFactory_threadsafety_bug.patch
>
>
> PhoneticFilterFactory has a static HashMap registry mapping an encoder name 
> to an implementation. There is a ReentrantLock used when the map is modified 
> (when the encoder config specifies a class name).  However, this map, which 
> can be accessed by multiple indexing threads, isn't guarded on any of the 
> reads, which isn't just the common path but also the error messages which 
> dump the registry into the error message.
> I realize the likelihood of a problem is extremely slim, but a bug's a bug.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to