[
https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477604
]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
>> Then, the error is something strange (eg confusing NullPointerException)
>> which doesn't make it clear what's happened.
>
> I think that depends on context. Many cases of NPEs are perfectly clear when
> you look at the stack trace.
Well for people familiar with Lucene's sources, yes. But most of our
users are not and so seeing an "AlreadyClosedException" can make a big
difference over [possibly rather nested, and, rather delayed] NPE or
something else.
EG look at the poor user that led to my opening this issue (link in
opening comment). The user was understandably confused by the NPE
inside the RAMDirectory.
> Adding ensureOpen() to something like maxDoc() does not ensure
> correctness though - an exception may or may not be thrown in the
> reader is already closed (because of those thread-safety issues). It
> actually increases the variability of behavior. We need to be
> careful not to guarantee the throwing of the exception.
On the thread safety issue: are you saying if one thread closes the
reader while another thread is using it, there is uncertainty excactly
when the 2nd thread will hit the AlreadyClosedException (because of
how the JVM schedules the threads)? I think this kind of thread
behavior is normal/expected?
Or is the thread safety issue something else?
>> especially common seems to be iterating through Hits after the reader is
>> closed
>
> Good point, for document() esp. I'm OK with the heavyweight methods.
>
>> Maybe we could remove checking for clearly frequently called methods (eg
>> isDeleted)?
>
> That's one of the ones I had in mind... isDeleted() can be called millions of
> times for a single query. Probably numDoc(), maxDoc(), etc, are also
> candidates.
OK how about we do not call ensureOpen() in these IndexReader methods?:
numDoc()
maxDoc()
isDeleted()
I think even without checking in those methods we still catch a number
of common cases:
* Close reader then try to run a search (termDocs()/termPositions()
catch the close)
* Run a search, close reader, then try to iterate through Hits
(document() catches the close)
* Close a reader then try to delete docs or setNorms
(deleteDocument()/setNorm() catch the close)
> Earlier detection of bugs when the cost is nil is good though...
Yes I think we in general want "fail-fast" here.
> what about setting more things to null when a reader is closed?
Well ... I would prefer not to increase the frequency of getting
"undefined" NPEs out of the reader. If we are going to cause
additional exceptions here, I'd like to make them intelligible to the
user (ie, AlreadyClosedException). EG, take SegmentReader's "si"
(SegmentInfo). If we set this to null on close, then numDoc() and
maxDoc() would hit NPE instead of just returning the correct answer.
I think I'd prefer returning the correct answer for such cases.
> IndexWriter should detect when it's used after being closed
> -----------------------------------------------------------
>
> Key: LUCENE-818
> URL: https://issues.apache.org/jira/browse/LUCENE-818
> 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
>
>
> Spinoff from this thread on java-user:
> http://www.gossamer-threads.com/lists/lucene/java-user/45986
> 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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
