Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-6651

Remove private field reflection (setAccessible) in AttributeImpl#reflectWith

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 5.2.1
    • 5.3, 6.0
    • core/other
    • None
    • New

    Description

      In AttributeImpl we currently have a "default" implementation of reflectWith (which is used by toString() and other methods) that uses reflection to list all private fields of the implementation class and reports them to the AttributeReflector (used by Solr and Elasticsearch to show analysis output).

      Unfortunately this default implementation needs to access private fields of a subclass, which does not work without doing Field#setAccessible(true). And this is done without AccessController#doPrivileged()!

      There are 2 solutions to solve this:

      • Reimplement the whole thing with MethodHandles. MethodHandles allow to access private fields, if you have a MethodHandles.Lookup object created from inside the subclass. The idea is to add a protected constructor taking a Lookup object (must come from same class). This Lookup object is then used to build methodHandles that can be executed to report the fields. Backside: We have to require subclasses that want this "automatic" reflection to pass a Lookup object in ctor's super(MethodHandles.lookup()) call. This breaks backwards for implementors of AttributeImpls
      • The second idea is to remove the whole reflectWith default impl and make the method abstract. This would require a bit more work in tons of AttributeImpl classes, but you already have to implement something like this for equals/hashCode, so its just listing all fields. This would of couse break backwards, too. So my plan would be to implement the missing methods everywhere (as if it were abstract), but keep the default implementation in 5.x. We just would do AccessController.doPrivileged().

      Attachments

        1. LUCENE-6651-MethodHandles.patch
          38 kB
          Uwe Schindler
        2. LUCENE-6651-5x.patch
          26 kB
          Uwe Schindler
        3. LUCENE-6651.patch
          22 kB
          Uwe Schindler
        4. LUCENE-6651.patch
          23 kB
          Uwe Schindler

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            uschindler Uwe Schindler
            uschindler Uwe Schindler
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment