Lucene - Core
  1. Lucene - Core
  2. LUCENE-2182

DEFAULT_ATTRIBUTE_FACTORY faills to load implementation class when iterface comes from different classloader

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.9.1, 3.0
    • Fix Version/s: 2.9.2, 3.0.1, 4.0-ALPHA
    • Component/s: core/other
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      This is a followup for http://www.lucidimagination.com/search/document/1724fcb3712bafba/using_the_new_tokenizer_api_from_a_jar_file:

      The DEFAULT_ATTRIBUTE_FACTORY should load the implementation class for a given attribute interface from the same classloader like the attribute interface. The current code loads it from the classloader of the lucene-core.jar file. In solr this fails when the interface is in a JAR file coming from the plugins folder.

      The interface is loaded correctly, because the addAttribute(FooAttribute.class) loads the FooAttribute.class from the plugin code and this with success. But as addAttribute tries to load the class from its local lucene-core.jar classloader it will not find the attribute.

      The fix is to tell Class.forName to use the classloader of the corresponding interface, which is the correct way to handle it, as the impl and the attribute should always be in the same classloader and file.

      I hope I can somehow add a test for that.

      1. LUCENE-2182.patch
        0.9 kB
        Uwe Schindler

        Activity

        Hide
        Uwe Schindler added a comment -

        The patch.

        Hello Solr committers: Should we backport this to 2.9 and 3.0 (which is easy)?

        Show
        Uwe Schindler added a comment - The patch. Hello Solr committers: Should we backport this to 2.9 and 3.0 (which is easy)?
        Hide
        Shalin Shekhar Mangar added a comment -

        In solr this fails when the interface is in a JAR file coming from the plugins folder

        Which interface are we talking about?

        Show
        Shalin Shekhar Mangar added a comment - In solr this fails when the interface is in a JAR file coming from the plugins folder Which interface are we talking about?
        Hide
        Uwe Schindler added a comment - - edited

        Shalin:

        If you use the new AttributeSource API for TokenStreams and define an own attribute (like the person did), you have to define two classes, see:
        http://lucene.apache.org/java/3_0_0/api/core/org/apache/lucene/analysis/package-summary.html -> New TokenStream API -> defining own attribute.

        You define an interface for your attribute and also define an implementation class. The TokenStream will call addAttribute(CustomAttribute.class) and the default attribute factory inside Lucene will then load the "default" implementation class "CustomAttributeImpl". This is done via reflection and Class.forName.

        If the interface is defined in an own Classloader (like the interface and impl is put into the solr plugins folder), the interface itsself is loaded correctly (because instantiated by the plugin's tokenstream code). But the code inside AttributeSource then tries to load the Impl class with forName(attributeClassName + "Impl"). As forName per default uses the classloader from the calling class, this fails, because th classloader lucene was loaded with does not know the plugins folder.

        Therefore, the implementation class must be loaded with the same classloader as the interface Class<? extends Attribute> object passed into addAttribute().

        Thats not a failure of Solr, but it the mechanism prevents putting own attribute impls into plugins of solr. They must be in the lib folder where lucene-core.jar is. By this patch it is more consistent and enforces, that the impl class comes from the same source as the base attribute.

        Show
        Uwe Schindler added a comment - - edited Shalin: If you use the new AttributeSource API for TokenStreams and define an own attribute (like the person did), you have to define two classes, see: http://lucene.apache.org/java/3_0_0/api/core/org/apache/lucene/analysis/package-summary.html -> New TokenStream API -> defining own attribute. You define an interface for your attribute and also define an implementation class. The TokenStream will call addAttribute(CustomAttribute.class) and the default attribute factory inside Lucene will then load the "default" implementation class "CustomAttributeImpl". This is done via reflection and Class.forName. If the interface is defined in an own Classloader (like the interface and impl is put into the solr plugins folder), the interface itsself is loaded correctly (because instantiated by the plugin's tokenstream code). But the code inside AttributeSource then tries to load the Impl class with forName(attributeClassName + "Impl"). As forName per default uses the classloader from the calling class, this fails, because th classloader lucene was loaded with does not know the plugins folder. Therefore, the implementation class must be loaded with the same classloader as the interface Class<? extends Attribute> object passed into addAttribute(). Thats not a failure of Solr, but it the mechanism prevents putting own attribute impls into plugins of solr. They must be in the lib folder where lucene-core.jar is. By this patch it is more consistent and enforces, that the impl class comes from the same source as the base attribute.
        Hide
        Shalin Shekhar Mangar added a comment -

        I see, thanks for the explanation!

        I agree, it would be nice if we can back port this to 2.9 if you have some spare cycles.

        Show
        Shalin Shekhar Mangar added a comment - I see, thanks for the explanation! I agree, it would be nice if we can back port this to 2.9 if you have some spare cycles.
        Hide
        Michael Busch added a comment -

        Looks like a good solution!

        Thanks for taking care of this, Uwe!

        Should we backport this to 2.9 and 3.0 (which is easy)?

        +1

        Show
        Michael Busch added a comment - Looks like a good solution! Thanks for taking care of this, Uwe! Should we backport this to 2.9 and 3.0 (which is easy)? +1
        Hide
        Uwe Schindler added a comment -

        I will commit and backport tomorrow!

        Show
        Uwe Schindler added a comment - I will commit and backport tomorrow!
        Hide
        Uwe Schindler added a comment -

        Committed trunk revision: 894348
        Committed Lucene 3.0 revision: 894351
        Committed Lucene 2.9 revision: 894352

        Show
        Uwe Schindler added a comment - Committed trunk revision: 894348 Committed Lucene 3.0 revision: 894351 Committed Lucene 2.9 revision: 894352

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development