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

IndexReader and friends should check ref count when incrementing

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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
        mikemccand 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
        mikemccand 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
        simonw Simon Willnauer added a comment -

        here is a patch

        Show
        simonw Simon Willnauer added a comment - here is a patch
        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

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

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

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

        agreed - uploaded a new patch

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

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

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

        +1

        Show
        mikemccand Michael McCandless added a comment - +1
        Hide
        jira-bot 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
        jira-bot 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
        jira-bot 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
        jira-bot 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:
            simonw Simon Willnauer
            Reporter:
            simonw Simon Willnauer
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development