lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael McCandless (JIRA)" <>
Subject [jira] Commented: (LUCENE-818) IndexWriter should detect when it's used after being closed
Date Tue, 06 Mar 2007 09:48:24 GMT


Michael McCandless commented on LUCENE-818:

> 2) the body of the catch clauses could be put into a helper method just like ensureOpen
to help reduce code noise

True, but I think it's still more code noise to have
try/catch/call-helper-method than ensureOpen() call?  Esp. since you'd
have to handle IOException and RuntimeException as 2 separate catch
clauses (I think?), each of which calls a separate helper.  Ie instead

  ensureOpen(); stuff...

we would need something like:

  try { stuff...
  } catch (IOException exc) {
    throw ensureOpenAfterIOException(exc);
  } catch (RuntimeException exc) {
    throw ensureOpenAfterRuntimeException(exc);

I think?

> 3) if there are situations where damage will be done by not testing that we are open
before taking some action, that would fall under my "adding better error checking in those
specific cases (if we know of any) and throwing explicit exceptions." ... a lot of this could
be achieved (as Yonik suggested) by nulling out more things in close so that the first attempt
to do something dangerous after the close triggered a NullPointerException.

The thing is we may not know all such cases (yet)?  I prefer taking a
defensive approach here.  I don't really like the null-out solution
because I think getting an AlreadyClosedException is clearer to the
user than a NPE's.

> 4) "fail fast" is always good ... except when it makes the non-failure case slow ...
i was merely suggesting an alternative that would achieve the same results without penalizing
performance of people obeying the rules.
> if numDoc(), maxDoc(), isDeleted(), and hasDeletions() are the only mehtods were people
are concerned about the performance impacts of calling ensureOpen() everytime, then those
methods could be the ones where isOpen could be checked in any exception handling block, and
all of the other mehtods could use ensureOpen as orriginal described.

Good idea!  An added bonus is that these methods do not throw
IOException so the exception handling would just have the one
RuntimeException catch clause above.  OK I will rework patch with this
approach and see how it looks...

> IndexWriter should detect when it's used after being closed
> -----------------------------------------------------------
>                 Key: LUCENE-818
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Index
>    Affects Versions: 2.1
>            Reporter: Michael McCandless
>         Assigned To: Michael McCandless
>            Priority: Minor
>         Attachments: LUCENE-818.patch, LUCENE-818.take2.patch, LUCENE-818.take3.patch
> Spinoff from this thread on java-user:
> If you call addDocument on IndexWriter after it's closed you'll hit a
> hard-to-explain NullPointerException (because the RAMDirectory was
> closed).  Before 2.1, apparently you won't hit any exception and the
> IndexWrite will keep running but will have released it's write lock (I
> think).
> I plan to fix IndexWriter methods to throw an IllegalStateException if
> it has been closed.

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:
For additional commands, e-mail:

View raw message