Lucene - Core
  1. Lucene - Core
  2. LUCENE-2030

CachingSpanFilter synchronizing on a none final protected object

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.9
    • Fix Version/s: 3.0
    • Component/s: core/search
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      CachingSpanFilter and CachingWrapperFilter expose their internal cache via a protected member which is lazily instantiated in the getDocSetId method. The current code yields the chance to double instantiate the cache and internally synchronizes on a protected none final member. My first guess is that this member was exposed for testing purposes so it should rather be changed to package private.

      This patch breaks backwards compat while I guess the cleanup is kind of worth breaking it.

      1. LUCENE-2030.patch
        5 kB
        Uwe Schindler
      2. LUCENE-2030.patch
        4 kB
        Uwe Schindler
      3. LUCENE-2030.patch
        3 kB
        Uwe Schindler
      4. LUCENE-2030.patch
        3 kB
        Simon Willnauer

        Activity

        Hide
        Uwe Schindler added a comment -

        This patch breaks serialization because the de-serializer does not call ctors and so the cache keeps unintialized (=null).

        Show
        Uwe Schindler added a comment - This patch breaks serialization because the de-serializer does not call ctors and so the cache keeps unintialized (=null).
        Hide
        Uwe Schindler added a comment -

        Fixed patch.

        Show
        Uwe Schindler added a comment - Fixed patch.
        Hide
        Simon Willnauer added a comment -

        Uwe, I'm so glad that you are so keen on stuff like Java serialization! Thanks

        Show
        Simon Willnauer added a comment - Uwe, I'm so glad that you are so keen on stuff like Java serialization! Thanks
        Hide
        Uwe Schindler added a comment -

        New patch. The previous one had the problem, that it also blocked during creating the Filter/SpanResult. Now it only locks correctly before creating the WeakHashMap at all and when requesting/putting entries. The only problem is, that two threads may create the same DocIdSet at the same time because of cache miss, but that is not a problem at all for correct behaviour.

        Show
        Uwe Schindler added a comment - New patch. The previous one had the problem, that it also blocked during creating the Filter/SpanResult. Now it only locks correctly before creating the WeakHashMap at all and when requesting/putting entries. The only problem is, that two threads may create the same DocIdSet at the same time because of cache miss, but that is not a problem at all for correct behaviour.
        Hide
        Uwe Schindler added a comment -

        New patch that uses ReentrantLock from Java5's concurrent package. This lock is serializable. All tests pass, will commit soon.

        Show
        Uwe Schindler added a comment - New patch that uses ReentrantLock from Java5's concurrent package. This lock is serializable. All tests pass, will commit soon.
        Hide
        Uwe Schindler added a comment -

        Committed revision: 833934

        Show
        Uwe Schindler added a comment - Committed revision: 833934

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development