Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Closeable.close() says "If the stream is already closed then invoking this method has no effect.".

      But a lot of our code does not really respect that. If i add an "extra" close() call in assertingcodec, it finds all kinds of bugs in codec code, for example:

         [junit4] Tests with failures (first 10 out of 59):
         [junit4]   - org.apache.lucene.index.TestCrashCausesCorruptIndex.testCrashCorruptsIndexing
         [junit4]   - org.apache.lucene.codecs.asserting.TestAssertingPostingsFormat.testDocsOnly
         [junit4]   - org.apache.lucene.codecs.asserting.TestAssertingPostingsFormat.testDocsAndFreqsAndPositionsAndOffsetsAndPayloads
         [junit4]   - org.apache.lucene.codecs.asserting.TestAssertingPostingsFormat.testDocsAndFreqs
         [junit4]   - org.apache.lucene.codecs.asserting.TestAssertingPostingsFormat.testDocsAndFreqsAndPositionsAndOffsets
         [junit4]   - org.apache.lucene.codecs.asserting.TestAssertingPostingsFormat.testRandom
         [junit4]   - org.apache.lucene.codecs.asserting.TestAssertingPostingsFormat.testDocsAndFreqsAndPositionsAndPayloads
         [junit4]   - org.apache.lucene.codecs.asserting.TestAssertingPostingsFormat.testDocsAndFreqsAndPositions
         [junit4]   - org.apache.lucene.index.TestDirectoryReader.testFilesOpenClose
         [junit4]   - org.apache.lucene.index.TestIndexWriterDelete.testIndexingThenDeleting
      
      1. LUCENE-6124_asserts.patch
        5 kB
        Robert Muir
      2. LUCENE-6124_asserts.patch
        3 kB
        Robert Muir
      3. LUCENE-6124.patch
        33 kB
        Robert Muir
      4. LUCENE-6124.patch
        23 kB
        Robert Muir
      5. LUCENE-6124.patch
        22 kB
        Robert Muir

        Activity

        Hide
        anshumg Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        anshumg Anshum Gupta added a comment - Bulk close after 5.0 release.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1646899 from Robert Muir in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1646899 ]

        LUCENE-6124: fix double-close bug in BaseDirectoryWrapper

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1646899 from Robert Muir in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1646899 ] LUCENE-6124 : fix double-close bug in BaseDirectoryWrapper
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1646898 from Robert Muir in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1646898 ]

        LUCENE-6124: fix double-close() bugs in WindowsFS etc

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1646898 from Robert Muir in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1646898 ] LUCENE-6124 : fix double-close() bugs in WindowsFS etc
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1646897 from Robert Muir in branch 'dev/trunk'
        [ https://svn.apache.org/r1646897 ]

        LUCENE-6124: fix double-close bug in BaseDirectoryWrapper

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1646897 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1646897 ] LUCENE-6124 : fix double-close bug in BaseDirectoryWrapper
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1646889 from Robert Muir in branch 'dev/trunk'
        [ https://svn.apache.org/r1646889 ]

        LUCENE-6124: fix double-close() bugs in WindowsFS etc

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1646889 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1646889 ] LUCENE-6124 : fix double-close() bugs in WindowsFS etc
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1646868 from Robert Muir in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1646868 ]

        LUCENE-6124: fix broken close() methods in .store/.codec apis

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1646868 from Robert Muir in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1646868 ] LUCENE-6124 : fix broken close() methods in .store/.codec apis
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1646854 from Robert Muir in branch 'dev/trunk'
        [ https://svn.apache.org/r1646854 ]

        LUCENE-6124: fix broken close() methods in .store/.codec apis

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1646854 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1646854 ] LUCENE-6124 : fix broken close() methods in .store/.codec apis
        Hide
        mikemccand Michael McCandless added a comment -

        +1

        Show
        mikemccand Michael McCandless added a comment - +1
        Hide
        jpountz Adrien Grand added a comment -

        +1

        Show
        jpountz Adrien Grand added a comment - +1
        Hide
        rcmuir Robert Muir added a comment -

        I generalized the method to test all Closeables in BaseIndexFileFormatTestCase.... then fixed all the problems.

        This is ready, i dont like some of the failures to be honest, because if you call close() again it will write extra data and create a corrupt file today.

        Show
        rcmuir Robert Muir added a comment - I generalized the method to test all Closeables in BaseIndexFileFormatTestCase.... then fixed all the problems. This is ready, i dont like some of the failures to be honest, because if you call close() again it will write extra data and create a corrupt file today.
        Hide
        rcmuir Robert Muir added a comment -

        added asserts to BasePostingsFormatTestCase. Now lots of codecs are angry

        Show
        rcmuir Robert Muir added a comment - added asserts to BasePostingsFormatTestCase. Now lots of codecs are angry
        Hide
        rcmuir Robert Muir added a comment -

        Here is current patch, with those tests passing. BlockTree, NRTCachingDir, MockDir, etc had bugs.

        We need to add checks in e.g. BasePostingsFormatTestCase and all those kinds of tests too, because I know there are other term dictionaries with the same pattern blocktree has.

        Show
        rcmuir Robert Muir added a comment - Here is current patch, with those tests passing. BlockTree, NRTCachingDir, MockDir, etc had bugs. We need to add checks in e.g. BasePostingsFormatTestCase and all those kinds of tests too, because I know there are other term dictionaries with the same pattern blocktree has.
        Hide
        rcmuir Robert Muir added a comment -

        I added tests to BaseDirectoryTestCase.

        Some .store code has bugs too, e.g.:

        org.apache.lucene.store.AlreadyClosedException: this Directory is closed
            at org.apache.lucene.store.BaseDirectory.ensureOpen(BaseDirectory.java:50)
            at org.apache.lucene.store.RAMDirectory.listAll(RAMDirectory.java:106)
            at org.apache.lucene.store.NRTCachingDirectory.close(NRTCachingDirectory.java:203)
            at org.apache.lucene.store.BaseDirectoryTestCase.testDoubleCloseDirectory(BaseDirectoryTestCase.java:1050)
        
        Show
        rcmuir Robert Muir added a comment - I added tests to BaseDirectoryTestCase. Some .store code has bugs too, e.g.: org.apache.lucene.store.AlreadyClosedException: this Directory is closed at org.apache.lucene.store.BaseDirectory.ensureOpen(BaseDirectory.java:50) at org.apache.lucene.store.RAMDirectory.listAll(RAMDirectory.java:106) at org.apache.lucene.store.NRTCachingDirectory.close(NRTCachingDirectory.java:203) at org.apache.lucene.store.BaseDirectoryTestCase.testDoubleCloseDirectory(BaseDirectoryTestCase.java:1050)
        Hide
        rcmuir Robert Muir added a comment -

        Here is the simple change i added to asserting codec... these are all bugs. And we should think about other things we could test too.

        Maybe BaseXXXTestCase can do it, to have better coverage across all codecs (and backwards ones). We should also think about BaseDirectoryTestCase for .store apis, and similar.

        Show
        rcmuir Robert Muir added a comment - Here is the simple change i added to asserting codec... these are all bugs. And we should think about other things we could test too. Maybe BaseXXXTestCase can do it, to have better coverage across all codecs (and backwards ones). We should also think about BaseDirectoryTestCase for .store apis, and similar.

          People

          • Assignee:
            Unassigned
            Reporter:
            rcmuir Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development