lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SOLR-3424) PhoneticFilterFactory threadsafety bug
Date Mon, 30 Apr 2012 09:01:45 GMT

    [ 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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message