Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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
        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
        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.
        Hide
        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
        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
        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
        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
        Robert Muir added a comment -

        added asserts to BasePostingsFormatTestCase. Now lots of codecs are angry

        Show
        Robert Muir added a comment - added asserts to BasePostingsFormatTestCase. Now lots of codecs are angry
        Hide
        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
        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
        Adrien Grand added a comment -

        +1

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

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development