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

Lucene fails to close file handles under certain situations

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.2
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      As a followon to LUCENE-820, I've added a further check in
      MockRAMDirectory to assert that there are no open files when the
      directory is closed.

      That check caused a few unit tests to fail, and in digging into the
      reason I uncovered these cases where Lucene fails to close file
      handles:

      • TermInfosReader.close() was setting its ThreadLocal enumerators to
        null without first closing the SegmentTermEnum in there. It looks
        like this was part of the fix for LUCENE-436. I just added the
        call to close.

      This is somewhat severe since we could leak many file handles for
      use cases that burn through threads and/or indexes. Though,
      FSIndexInput does have a finalize() to close itself.

      • Flushing of deletes in IndexWriter opens SegmentReader to do the
        flushing, and it correctly calls close() to close the reader. But
        if an exception is hit during commit and before actually closing,
        it will leave open those handles. I fixed this first calling
        doCommit() and then doClose() in a finally. The "disk full" tests
        we now have were hitting this.
      • IndexWriter's addIndexes(IndexReader[]) method was opening a
        reader but not closing it with a try/finally. I just put a
        try/finally in.

      I've also changed some unit tests to use MockRAMDirectory instead of
      RAMDirectory to increase testing coverage of "leaking open file
      handles".

      1. LUCENE-823.patch
        18 kB
        Michael McCandless
      2. LUCENE-823.take2.patch
        20 kB
        Michael McCandless

        Activity

        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

        > TermInfosReader.close() was setting its ThreadLocal enumerators to
        > null without first closing the SegmentTermEnum in there.

        Are you sure descriptors are really leaked in this case?
        There are other cases where Lucene doesn't close IndexInput clones... see LUCENE-686

        The SegmentTermEnum constructor is only called in two places:

        • to init TermInfosReader.origEnum
        • to init TermInfosReader.indexEnum

        We can therefore conclude that all other SegmentTermEnum instances are clones, and hence close is a no-op.

        Show
        yseeley@gmail.com Yonik Seeley added a comment - > TermInfosReader.close() was setting its ThreadLocal enumerators to > null without first closing the SegmentTermEnum in there. Are you sure descriptors are really leaked in this case? There are other cases where Lucene doesn't close IndexInput clones... see LUCENE-686 The SegmentTermEnum constructor is only called in two places: to init TermInfosReader.origEnum to init TermInfosReader.indexEnum We can therefore conclude that all other SegmentTermEnum instances are clones, and hence close is a no-op.
        Hide
        mikemccand Michael McCandless added a comment -

        Ahh, tricky!

        You are right, this is not a real leak, since it's a clone that's
        failing to be closed and since FSIndexInput does not make a clone of
        the underlying file descriptor.

        I had assumed/expected that a clone must be closed, but I guess it's
        [currently] a no-op. This is the heart of the debate in LUCENE-686.

        I guess I'll leave the close in there, but change MockRAMDirectory to
        ignore still-open clones when the dir is closed and add a comment
        about this LUCENE-686.

        Show
        mikemccand Michael McCandless added a comment - Ahh, tricky! You are right, this is not a real leak, since it's a clone that's failing to be closed and since FSIndexInput does not make a clone of the underlying file descriptor. I had assumed/expected that a clone must be closed, but I guess it's [currently] a no-op. This is the heart of the debate in LUCENE-686 . I guess I'll leave the close in there, but change MockRAMDirectory to ignore still-open clones when the dir is closed and add a comment about this LUCENE-686 .
        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

        > guess I'll leave the close in there

        Since it's a thread-local, that only closes (maybe) for a single thread.
        Doesn't seem worth it, and could be misleading to anyone thinking it closed all the enumerators.

        Show
        yseeley@gmail.com Yonik Seeley added a comment - > guess I'll leave the close in there Since it's a thread-local, that only closes (maybe) for a single thread. Doesn't seem worth it, and could be misleading to anyone thinking it closed all the enumerators.
        Hide
        mikemccand Michael McCandless added a comment -

        > Since it's a thread-local, that only closes (maybe) for a single thread.
        > Doesn't seem worth it, and could be misleading to anyone thinking it closed all the enumerators.

        Good point. OK, I won't add the close call in.

        Show
        mikemccand Michael McCandless added a comment - > Since it's a thread-local, that only closes (maybe) for a single thread. > Doesn't seem worth it, and could be misleading to anyone thinking it closed all the enumerators. Good point. OK, I won't add the close call in.
        Hide
        mikemccand Michael McCandless added a comment -

        Fixed patch based on Yonik's comments (thanks for the review!).

        Show
        mikemccand Michael McCandless added a comment - Fixed patch based on Yonik's comments (thanks for the review!).

          People

          • Assignee:
            mikemccand Michael McCandless
            Reporter:
            mikemccand Michael McCandless
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development