Lucene - Core
  1. Lucene - Core
  2. LUCENE-5362

IndexReader and friends should check ref count when incrementing

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.6
    • Fix Version/s: 4.7, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      IndexReader and SegmentCoreReaders blindly increments it's refcount which could already be counted down to 0 which might allow an IndexReader to "rise from the dead" and use an already closed SCR instance. Even if that is caught we should try best effort to raise ACE asap.

      1. LUCENE-5362.patch
        2 kB
        Simon Willnauer
      2. LUCENE-5362.patch
        2 kB
        Simon Willnauer

        Activity

        Hide
        Michael McCandless added a comment -

        It's somewhat crazy for an app to be closing AND trying to reopen at the same time (i.e. results can be "unpredictable", just like in IW), but I agree we should make a best effort here if the code changes are minor.

        I think we can just change incRef to call tryIncRef, and then throw ACE if tryIncRef returns false?

        Show
        Michael McCandless added a comment - It's somewhat crazy for an app to be closing AND trying to reopen at the same time (i.e. results can be "unpredictable", just like in IW), but I agree we should make a best effort here if the code changes are minor. I think we can just change incRef to call tryIncRef, and then throw ACE if tryIncRef returns false?
        Hide
        Simon Willnauer added a comment -

        here is a patch

        Show
        Simon Willnauer added a comment - here is a patch
        Hide
        Yonik Seeley added a comment -

        The first ensureOpen() in incRef() now seems redundant after this patch?

        Show
        Yonik Seeley added a comment - The first ensureOpen() in incRef() now seems redundant after this patch?
        Hide
        Simon Willnauer added a comment -

        The first ensureOpen() in incRef() now seems redundant after this patch?

        agreed - uploaded a new patch

        Show
        Simon Willnauer added a comment - The first ensureOpen() in incRef() now seems redundant after this patch? agreed - uploaded a new patch
        Hide
        Uwe Schindler added a comment -

        +1, looks correct. SegemntCoreReaders atomic increment is also correct - @BrianGoetzSays

        Show
        Uwe Schindler added a comment - +1, looks correct. SegemntCoreReaders atomic increment is also correct - @BrianGoetzSays
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        ASF subversion and git services added a comment -

        Commit 1549012 from Simon Willnauer in branch 'dev/trunk'
        [ https://svn.apache.org/r1549012 ]

        LUCENE-5362: IndexReader and SegmentCoreReaders now throw AlreadyClosedException if the refCount in incremented but is less that 1.

        Show
        ASF subversion and git services added a comment - Commit 1549012 from Simon Willnauer in branch 'dev/trunk' [ https://svn.apache.org/r1549012 ] LUCENE-5362 : IndexReader and SegmentCoreReaders now throw AlreadyClosedException if the refCount in incremented but is less that 1.
        Hide
        ASF subversion and git services added a comment -

        Commit 1549013 from Simon Willnauer in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1549013 ]

        LUCENE-5362: IndexReader and SegmentCoreReaders now throw AlreadyClosedException if the refCount in incremented but is less that 1.

        Show
        ASF subversion and git services added a comment - Commit 1549013 from Simon Willnauer in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1549013 ] LUCENE-5362 : IndexReader and SegmentCoreReaders now throw AlreadyClosedException if the refCount in incremented but is less that 1.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development