Mailing-List: contact java-dev-help@lucene.apache.org; run by ezmlm
Precedence: bulk
Reply-To: java-dev@lucene.apache.org
Message-ID: <1693182340.403001269264807198.JavaMail.jira@brutus.apache.org>
Date: Mon, 22 Mar 2010 13:33:27 +0000 (UTC)
From: "Mike Hanafey (JIRA)" <jira@apache.org>
To: java-dev@lucene.apache.org
Subject: [jira] Commented: (LUCENE-2334) IndexReader.close() should call
 IndexReader.decRef() unconditionally ??
In-Reply-To: <1656827675.364921269010888131.JavaMail.jira@brutus.apache.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/LUCENE-2334?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12848122#action_12848122 ] 

Mike Hanafey commented on LUCENE-2334:
--------------------------------------

The problem is IndexReader implements the Closable interface whose contract is:
{code}    /**
     * Closes this stream and releases any system resources associated
     * with it. If the stream is already closed then invoking this 
     * method has no effect. 
     *
     * @throws IOException if an I/O error occurs
     */
{code}

The JavaDoc in IndexReader suggests the Closable index is implemented as per the contract:
{code}  /**
   * Closes files associated with this index.
   * Also saves any new deletions to disk.
   * No other methods should be called after this has been called.
   * @throws IOException if there is a low-level IO error
   */
{code}

This means a user of the API should be certain that one call to close actually does free the system resources.

My suggestion that close() should always defRef() was by analogy with database connection pooling where the calling code works whether or not connections are pooled. When done with the connection it is just closed and the pool decides if and when the closing actually happens. But admittedly it a perversion to call this and index reader pool.

With the current implementation of IndexReader, the first close() behaves one way (decRef()), but subsequent ones do not, which means the caller of close() must know if this is a "simple" or "advanced" use case, so then why not just be direct - close() means close for sure, and if it is a muti-thread type usage close() is a mistake and decRef() should be used. As written the advanced usage case means lots of extra file handles are tied up and some day the app server dies with a "too many file handles" problem, but if close() really meant close then the advanced usage case means an exception occurs very early in testing. (As you undoubtly guessed, not hypothetical -- but this is the reason I even looked at this part of the code!)


> IndexReader.close() should call IndexReader.decRef() unconditionally ??
> -----------------------------------------------------------------------
>
>                 Key: LUCENE-2334
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2334
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>    Affects Versions: 3.0.1
>            Reporter: Mike Hanafey
>            Priority: Minor
>
> IndexReader.close() is defined:
> {code}  /**
>    * Closes files associated with this index.
>    * Also saves any new deletions to disk.
>    * No other methods should be called after this has been called.
>    * @throws IOException if there is a low-level IO error
>    */
>   public final synchronized void close() throws IOException {
>     if (!closed) {
>       decRef();
>       closed = true;
>     }
>   }
> {code}
> This  means that  if the refCount is bigger than one, close() does not actually close, but it is also true that calling close() again has no effect.
> Why does close() not simply call decRef() unconditionally? This way if incRef() is called each time an instance of IndexReader were handed out, if close() is called by each recipient when they are done, the last one to call close will actually close the index. As written it seems the API is very confusing -- the first close() does one thing, but the next close() does something different.
> At a minimum the JavaDoc should clarify the behavior.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org