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

Clarify documentation of clone() in IndexInput

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.3, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Here is a snippet from IndexInput's documentation:

      The original instance must take care that cloned instances throw AlreadyClosedException when the original one is closed.
      

      But concrete implementations don't throw this AlreadyClosedException (this would break the contract on Closeable). For example, see NIOFSDirectory:

          public void close() throws IOException {
            if (!isClone) {
              channel.close();
            }
          }
      

      What trapped me was that the abstract class IndexInput overrides the default implementation of clone(), but doesn't do anything useful... I guess you could make it final and provide the tracking for cloned instances in this class rather than reimplementing it everywhere else (isCloned() would be a superclass method then too). Thoughts?

        Activity

        Hide
        thetaphi Uwe Schindler added a comment -

        According to "java.io.Closeable" docs, closing should not throw exceptions because multiple closes are allowed (so if it implicitely closed by root object, an additional call to clone's close should not fail).

        The comment means: If you access the cloned IndexInput after closing the original the "readXXX" methods will throw AlreadyClosedException. For clones. the close() method is a noop, that is intended.

        Show
        thetaphi Uwe Schindler added a comment - According to "java.io.Closeable" docs, closing should not throw exceptions because multiple closes are allowed (so if it implicitely closed by root object, an additional call to clone's close should not fail). The comment means: If you access the cloned IndexInput after closing the original the "readXXX" methods will throw AlreadyClosedException. For clones. the close() method is a noop, that is intended.
        Hide
        dweiss Dawid Weiss added a comment -

        I think the comment should read exactly what your explanation of the comment was, it'd be clearer then...

        Show
        dweiss Dawid Weiss added a comment - I think the comment should read exactly what your explanation of the comment was, it'd be clearer then...
        Hide
        dweiss Dawid Weiss added a comment -

        Uwe, this comment is effectively dead, isn't it? I mean – looking at NIOFSIndexInput for example, its isClone field is never used. So it's very likely that this contract is violated?

        Show
        dweiss Dawid Weiss added a comment - Uwe, this comment is effectively dead, isn't it? I mean – looking at NIOFSIndexInput for example, its isClone field is never used. So it's very likely that this contract is violated?
        Hide
        dweiss Dawid Weiss added a comment -

        Oh, ok. Nevermind, the original source would throw ACE.

        Show
        dweiss Dawid Weiss added a comment - Oh, ok. Nevermind, the original source would throw ACE.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1691888 from Dawid Weiss in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1691888 ]

        LUCENE-6225: Clarify documentation of clone/close in IndexInput.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1691888 from Dawid Weiss in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1691888 ] LUCENE-6225 : Clarify documentation of clone/close in IndexInput.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1691892 from Dawid Weiss in branch 'dev/trunk'
        [ https://svn.apache.org/r1691892 ]

        LUCENE-6225: Clarify documentation of clone/close in IndexInput.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1691892 from Dawid Weiss in branch 'dev/trunk' [ https://svn.apache.org/r1691892 ] LUCENE-6225 : Clarify documentation of clone/close in IndexInput.
        Hide
        shalinmangar Shalin Shekhar Mangar added a comment -

        Bulk close for 5.3.0 release

        Show
        shalinmangar Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

          People

          • Assignee:
            dweiss Dawid Weiss
            Reporter:
            dweiss Dawid Weiss
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development