You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Michael McCandless (JIRA)" <ji...@apache.org> on 2007/02/26 20:34:07 UTC
[jira] Created: (LUCENE-818) IndexWriter should detect when it's
used after being closed
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
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless updated LUCENE-818:
--------------------------------------
Attachment: LUCENE-818.take4.patch
New patch based on above discussion: just re-based to the current trunk, and, added comments on why ensureOpen is not called in the 4 methods.
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12478166 ]
Hoss Man commented on LUCENE-818:
---------------------------------
1) my example attempted to be concise ... ideally we would be explicit in our catches.
2) the body of the catch clauses could be put into a helper method just like ensureOpen to help reduce code noise
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.
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.
as an added bonus, both methodologies could be used...
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.
> 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, LUCENE-818.take3.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Steven Parkes (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12476373 ]
Steven Parkes commented on LUCENE-818:
--------------------------------------
Shades of LUCENE-686. It'd be nice to agree on that one, too (though it's internal, not external).
> 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
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477399 ]
Doug Cutting commented on LUCENE-818:
-------------------------------------
> Perhaps for close() of an already closed object just do nothing?
+1 Double-closing is sometimes hard to avoid. I'd rather have it ignored than force folks to, e.g., write more complicated exception handlers.
> 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
>
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless updated LUCENE-818:
--------------------------------------
Attachment: LUCENE-818.take5.patch
Attached patch: removed AlreadyClosedException from throws clauses & javadocs; added ensureOpen calls inside RAMDirectory as well.
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.patch, LUCENE-818.take5.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Daniel John Debrunner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477716 ]
Daniel John Debrunner commented on LUCENE-818:
----------------------------------------------
>> Yes, but it's not just thread scheduling, it's also lack of memory
>> barriers. The 2nd thread may *never* see the close(), depending on
>> the exact architecture of machine and the JVM.
>Yikes. Is this the Java memory model issue? Ie, there is no hard
>guarantee on when a "write" from one thread will be visible to other
>threads, unless you use "volatile"?
Doesn't crossing a synchronization barrier ensure that all threads will seem the same value of a field?
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477412 ]
Yonik Seeley commented on LUCENE-818:
-------------------------------------
I think that accessing a closed reader should continue to be undefined.
If we define it to throw an exception, then we have thread safety issues which would be too costly (IMO) to fix.
If we are to add checks for a closed reader, it seems like they should only be put on methods where an additional method call would be negligible. Hand-holding that decreases performance for everyone isn't something I like. Sun often goes too far in this regard, and as a consequence, people end up rewriting their own version of classes to get better performance.
After all, this is really just making nicer error messages for incorrect programs, right? Everyone shouldn't have to pay for that. I guess I'm a minimalist :-)
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Doron Cohen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477398 ]
Doron Cohen commented on LUCENE-818:
------------------------------------
Throwing that exception for close() seems a bit too restrictive...
Perhaps for close() of an already closed object just do nothing?
This would be more forgiving/friendly to "over protecting" (possibly existing) applications.
> 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
>
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless updated LUCENE-818:
--------------------------------------
Attachment: LUCENE-818.take2.patch
Excellent point. OK I fixed the patch to silently ignore close() when
the IndexReader, IndexWriter, FieldsReader was already closed.
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Doron Cohen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12479207 ]
Doron Cohen commented on LUCENE-818:
------------------------------------
> People shouldn't have specific application logic for this exception... it should be treated as a program error.
+1
Also, another candidate for AlreadyClosedException upon using an already closed (RAM)Directory - came up in http://www.nabble.com/sharing-my-experience-for-upgrading-from-Lucene-1.9-to-Lucene-2.2-dev-tf3366948.html
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12478544 ]
Hoss Man commented on LUCENE-818:
---------------------------------
i would make sure to *heavily* document each and every use of isOpen/ensureOpen to make it clear why they are being used when they are from a performance concern case ... otherwise people will be very confused down the road ("why the hell isn't this being checked until the catch block?!?!?!")
> 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, LUCENE-818.take3.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12476386 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
I'm planning on adding o.a.l.store.AlreadyClosedException.
But, I'm back to subclassing this from IllegalStateException (not
IOException). There are a fair number of methods where we should
throw this exception but IOException was not already in the throws
clause. Also, since it's rather rare, I think subclassing an
unchecked Exception is appropriate.
The only counter argument is IndexReader which currently throws
IOException if it has been closed but then acquireWriteLock is called
(for one of the "writer" methods). So this would technically be an
API change for this particular case in IndexReader. But I think it's
a tiny API change especially because it does not affect user's code
since they already must catch the existing IOException (vs the other
methods where adding checked "throws IOException" would be a major API
change).
> 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
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12479373 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
> I'm not enough of an expert to know if this would be faster or slower than a simple fileMap==null check.
> I would guess it depends on what needs to be set up in the stack frame to potentially catch an exception, and if the try/catch/throw code prevents any optimizations (such as inlining).
>
> Also, catching a NPE seems a little icky to me... so w/o more info I'd lean toward a fileMap==null check (if anything).
OK, good points. It is not in fact clear that a try/catch solution is
less costly, so, let's keep it simple. I will just add ensureOpen().
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12479357 ]
Yonik Seeley commented on LUCENE-818:
-------------------------------------
> we can use Hoss's neat approach and
> specifically catch the NPE and rethrow as an AlreadyClosedException?
I'm not enough of an expert to know if this would be faster or slower than a simple fileMap==null check.
I would guess it depends on what needs to be set up in the stack frame to potentially catch an exception, and if the try/catch/throw code prevents any optimizations (such as inlining).
Also, catching a NPE seems a little icky to me... so w/o more info I'd lean toward a fileMap==null check (if anything).
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477457 ]
Yonik Seeley 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.
> In the IndexWriter case it's potentially devastating
Agreed... when it affects correctness, we should fix.
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.
> 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.
Earlier detection of bugs when the cost is nil is good though...
what about setting more things to null when a reader is closed?
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477670 ]
Yonik Seeley commented on LUCENE-818:
-------------------------------------
> 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)?
Yes, but it's not just thread scheduling, it's also lack of memory barriers. The 2nd thread may *never* see the close(), depending on the exact architecture of machine and the JVM.
> I think this kind of thread behavior is normal/expected?
For a class that isn't thread safe, yes. IndexReader is advertised as being thread safe though. If we guarantee an exception accessing a closed reader, then that should work 100% of the time. I don't think we should make that guarantee.
We can still throw meaningful errors in more cases and make it easier for the user to debug that, but it should not be deemed an error if we don't throw an exception. Users should never rely on getting this exception for flow-control purposes anyway.
> OK how about we do not call ensureOpen() in these IndexReader methods?:
> numDoc()
> maxDoc()
> isDeleted()
+1
hasDeletions() too?
> > 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
Yes, but not all bugs will be user bugs. Some will be internal Lucene stuff that bypass public methods.
It's still better that these fail quicker too. Anyway, that can be handled on a case-by-case basis later.
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12476240 ]
Grant Ingersoll commented on LUCENE-818:
----------------------------------------
FWIW: Some of the other Readers (actually, IndexInput) are undefined when invoking after close. I ran into this w/ the FieldsReader when working on lazy loading. See http://www.gossamer-threads.com/lists/lucene/java-dev/34109?search_string=lazy%20field%20loading;#34109
for reference.
> 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
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477723 ]
Yonik Seeley commented on LUCENE-818:
-------------------------------------
> Doesn't crossing a synchronization barrier ensure that all threads will seem the same value of a field?
Yes, if the changing thread crosses a write barrier and the reading thread crosses a read barrier.
The obvious implication that you need to synchronize the sets/gets of isClosed. That's not a big deal in some cases, but in anything that could be in an inner loop, it is a big deal. We already have reports of simple things like isDeleted() being a bottleneck on servers with many CPUs and concurrent threads because of the synchronization. That particular sync point is something that I've planned to get rid of at some point in the future by having a "read only" IndexReader.
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12476044 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
That approach seems a little overkill to me.
I would need to create a new Closeable interface, and then a static
"ensureOpen" method somewhere else. These would probably live in
util, so then this is all public: users see that these classes
implement Closeable, yet, it's not really a publicly useful feature,
so it adds noise to the javadocs.
I was planning instead on just adding a private "ensureOpen()" to
IndexWriter (and I agree I should do IndexReader as well) that throws
IllegalStateException if it's closed. I think noise is kept to the
same minimum (?) because all public methods that require this just have an
added ensureOpen() call at the top. This is how IndexModifier works
today.
Hmm, IndexReader already throws IOException in certain cases if it's
already closed. So I think I will make a new exception
(AlreadyClosedException?) that subclasses IOException and throw that.
> 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
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12478585 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
> i would make sure to *heavily* document each and every use of isOpen/ensureOpen to make it clear why they are being used when they are from a performance concern case ... otherwise people will be very confused down the road ("why the hell isn't this being checked until the catch block?!?!?!")
OK I changed my mind.
I don't think we should add the "check if closed after the fact"
logic. I reviewed the 4 methods in question (maxDoc, numDocs,
hasDeletions, isDeleted) and for all subclasses of IndexReader these
are very trivial methods that won't throw exceptions.
So I'm back to thinking this added complexity (and future confusion to
people looking @ Lucene source code) isn't worthwhile and we should
leave these methods with no checking for whether they are already
closed.
But I will document why ensureOpen is *not* being called in each of
these methods.
> 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, LUCENE-818.take3.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477713 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
>> 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)?
>
> Yes, but it's not just thread scheduling, it's also lack of memory
> barriers. The 2nd thread may *never* see the close(), depending on
> the exact architecture of machine and the JVM.
Yikes. Is this the Java memory model issue? Ie, there is no hard
guarantee on when a "write" from one thread will be visible to other
threads, unless you use "volatile"?
>> I think this kind of thread behavior is normal/expected?
>
> For a class that isn't thread safe, yes. IndexReader is advertised
> as being thread safe though. If we guarantee an exception accessing
> a closed reader, then that should work 100% of the time. I don't
> think we should make that guarantee.
OK I think we shouldn't "guarantee" it. I think listing as "@throws
AlreadyClosedException if this IndexReader is closed" is OK?
> We can still throw meaningful errors in more cases and make it
> easier for the user to debug that, but it should not be deemed an
> error if we don't throw an exception. Users should never rely on
> getting this exception for flow-control purposes anyway.
Agreed.
>> OK how about we do not call ensureOpen() in these IndexReader methods?:
>> numDoc()
>> maxDoc()
>> isDeleted()
>
> +1
>
> hasDeletions() too?
OK I will change to not call ensureOpen() for hasDeletions too. I
will roll a new patch with this.
>> > 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
>
> Yes, but not all bugs will be user bugs. Some will be internal
> Lucene stuff that bypass public methods. It's still better that
> these fail quicker too. Anyway, that can be handled on a
> case-by-case basis later.
OK, I agree. Better to throw a "fail-fast" NPE than do something
strange later.
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless updated LUCENE-818:
--------------------------------------
Fix Version/s: 2.2
> 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
> Fix For: 2.2
>
> Attachments: LUCENE-818.patch, LUCENE-818.take2.patch, LUCENE-818.take3.patch, LUCENE-818.take4.patch, LUCENE-818.take5.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless updated LUCENE-818:
--------------------------------------
Attachment: LUCENE-818.take3.patch
Attached take3, which just removes ensureOpen() checks for IndexReader's maxDoc(), numDoc(), hasDeletions(), isDeleted(...).
> 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, LUCENE-818.take3.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477724 ]
Yonik Seeley commented on LUCENE-818:
-------------------------------------
> Yikes. Is this the Java memory model issue?
Yes, but it's not a problem with the Java memory model, it's more of a feature. Threads in general have this problem (or perform very poorly if they don't). It's also very much about compiler optimizations as well. The optimizer may stuff a value in a register, and never check it's location in main-memory, and never see the changes. Even if it doesn't then you drop down to problems with the CPU and cache + memory architectue in SMP systems. Some systems require explicit memory barriers or cache flushes to see memory changes from another CPU. One CPU writes to a spot in memory, and absent any special instructions, another CPU reading from that spot in memory may not see the change.
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Resolved: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless resolved LUCENE-818.
---------------------------------------
Resolution: Fixed
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.patch, LUCENE-818.take5.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477442 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
Yes, this is about giving a clear (and prompt) error message when
Lucene is used incorrectly.
LUCENE-140 was also a similar situation. In that case incorrect usage
could lead to index corruption much later.
I feel like that is important because it affects the perceived quality
of Lucene. Enough users seem to hit this (especially common seems to
be iterating through Hits after the reader is closed). As Lucene
adoption grows, more users will hit it.
Because detection of this now is so "intermittant" (undefined), a
developer could think everything is OK in testing, push things into
their production world, only to suddenly see an error. Then, the
error is something strange (eg confusing NullPointerException) which
doesn't make it clear what's happened.
In the IndexWriter case it's potentially devastating because (I
think?) if you keep using your closed IndexWriter, then, it's
operating on the index without holding the write lock (this was pre
2.1).
I do agree we shouldn't go overboard with this (eg I certainly think
we should not add checks inside things like TermDocs/TermPositions).
And I agree this is in general a difficult tradeoff.
But I think in this case the performance impact is likely very small; I could
work out a test to be sure.
Maybe we could remove checking for clearly frequently called methods
(eg isDeleted)?
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless updated LUCENE-818:
--------------------------------------
Attachment: LUCENE-818.patch
Attached patch.
I added checks to all public methods in IndexReader (and its
subclasses in Lucene's sources: SegmentReader, MultiReader,
FilteredIndexReader, ParallelReader), IndexWriter and FieldsReader
that throw AlreadyClosedException if they are used after being closed.
I then hit a few tests that were incorrectly closing
reader/searcher/writer and then continuing to use them (or double
closing them), so I fixed those.
> 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
>
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12476360 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
I see, so IndexInput indeed does not track isClosed. It's not easy to
fix that since IndexInput is public and non-final?
Though I'm actually less concerned about this one because it's very
"internal" to Lucene.
I'm more concerned about the public paths (IndexWriter, IndexReader).
Users seem to accidentally close these classes fairly often. I think
IndexSearcher is OK because on close it at worst closes the underlying
IndexReader, and then any call to that IndexReader will catch that
it's closed.
Oh, I see: this public path will indeed access IndexInput after it's
been closed:
Open reader
Get a doc w/ lazy field(s)
Close reader
Try to get the lazy field's value
That last step will clone the cloneableFieldsStream after it had been
closed. Hmmm. Though I think we could just fix this path in
FieldsReader by having it record when it's closed & throw an
AlreadyClosedException if the LazyField class is used after that?
That seems like the simplest fix.
> 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
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477957 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
To make that work, I think we'd have to convert the Exception into
either IOException or RuntimeException before throwing it (so we don't
have to change method signature).
But, I think this approach adds more noise to the source code. Also
because it's "after the fact" then damage could have been done (I
*think* IndexWriter before 2.1 would gleefully keep writing, but
without holding the write lock!). It would only catch cases where an
exception was thrown (many times there is none) whereas I prefer "fail
fast".
> 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, LUCENE-818.take3.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Doron Cohen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12475987 ]
Doron Cohen commented on LUCENE-818:
------------------------------------
I looked at Java IO as a reference for acceptable behavior in this regard.
If you close a file system object and try to use it,
- RandomAccessFile will throw IOException with: "No such file or directory"
- FileWriter will throw IOException with "Stream closed"
- FileOutputStream will throw IOException with "Stream closed"
So you could say that Java did not go all the way with a well defined illegalState exception.
Databases on the other hand would give you a well definied error code.
I don't have a strong opinion if this is a must for Lucene or not.
But if this is to be added, one way to do it is to define a Closable interface { close(); isOpen() }, implement this interface in all public classes that need to check open state (IndexWriter, IndexReader, etc.), add a single static ensureOpen(Closable) method in a utils class that would throw the IllegalState exception, and just call this method from every (public?) open-state open dependent method. I think this would keep the 'noise' in the code to minimum.
> 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
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12478327 ]
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
of:
ensureOpen();
...do stuff...
we would need something like:
try {
...do 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: 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, LUCENE-818.take3.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477875 ]
Hoss Man commented on LUCENE-818:
---------------------------------
I havne't been following this issue that closely, and i haven't read any of the patches, but if the goal is:
"reduce situations where novice users get confusing exceptions when using IndexWriter after closing it"
...and the concern is that calling a new "ensureOpen()" method at the begining of every public method may be a performacne issue, then perhaps an alternative would be to catch all exceptions inside public methods and wrap them if not closed, ie...
public Foo somePublicMethod() throws IOException {
try {
// current method body
} catch (Exception e) {
if (isOpen()) {
throw e;
} else {
throw new AllreadyClosedException(e);
}
}
}
...that way any performance cost of ensureOpen/isOpen is only payed on an existing Exception. the down side i see to this approach is that since it only explains exceptions hte caller would get anyway, it doesn't give any feedback about using a closed writer in cases where the current code returns cleanly (but may not function properly) ... we might be able to solve that by adding better error checking in those specific cases (if we know of any) and throwing explicit exceptions.
> 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, LUCENE-818.take3.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12479179 ]
Yonik Seeley commented on LUCENE-818:
-------------------------------------
If we aren't guaranteeing an exception be thrown after a close, why are we bothering to put the exception in the method signature?
It's already apparent to everyone that using something after it's closed isn't good, so we were just cleaning up the error messages and making it easier to debug. People shouldn't have specific application logic for this exception... it should be treated as a program error.
IMO, listing the exception encourages people to catch it, suggests that they can depend on the exception being thrown, and also makes it more difficult to remove it from certain methods in the future.
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-818) IndexWriter should detect when it's
used after being closed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12479273 ]
Michael McCandless commented on LUCENE-818:
-------------------------------------------
>> People shouldn't have specific application logic for this exception... it should be treated as a program error.
> +1
OK, agreed. I will remove AlreadyClosedException from all @throws
method signatures.
> Also, another candidate for AlreadyClosedException upon using an already closed (RAM)Directory - came up in http://www.nabble.com/sharing-my-experience-for-upgrading-from-Lucene-1.9-to-Lucene-2.2-dev-tf3366948.html
Yeah I saw this too. Here's the relevant excerpt:
> 3. RAMDirectory related changes
> It took me something to find this out. Previously, after
> ramDirectory = new RAMDirectory(file)
> I could ramDirectory.close() to release the resources. And later, I
> could do a check for IndexReader.indexExists(ramDirectory) to see if
> there is an index in the directory. FSDirectory behaves this way also.
> But with lucene 2.2, NullPointerExceptions came out. It turns out
> when ramDirectory.close(), the instance variable fileMap is set to
> null. And IndexReader.indexExists(ramDirectory) is reading fileMap to
> look for indexes, causing the NPE.
In fact the original user's list email that started this bug was also
due to RAMDirectory setting its fileMap to null, but in that case, it
was via IndexWriter so detecting that IndexWriter is closed would
prevent that one.
In this case the developer is using a RAMDirectory directly.
I think this is an example where "nulling things out on close" leads
to developer confusion. Previously RAMDirectory functioned fine after
being closed(); now, it throws a hard to understand (unless you are
familiar w/ Lucene's sources & what specifically we changed in 2.1)
NPE.
I think we should fix this?
Since RAMDir's public methods are fairly hot (eg heavily used building
single-doc RAM segment), we can use Hoss's neat approach and
specifically catch the NPE and rethrow as an AlreadyClosedException?
I don't mind if the "after close semantics" is "it works just like it
did when it was open" (ie, close is a no-op). I also don't mind if
you get a "fail-fast" quick AlreadyClosedException. But I think
anything in between (NPE or other undefined, intermittant exceptions)
only confuses our developers.
Looking at FSDirectory, it continues to work fine after close with the
one spooky exception that it may have been removed from the
DIRECTORIES hashtable, which means if you then open the same canonical
path again you get a different FSDirectory instance. The comment
states "this permits synchronization on directories", but I don't see
where in Lucene we are relying on this? Ie, what could break if a
user keeps using a closed FSDirectory thus possibly having more than
one FSDirectory instance for a given canonical path?
> 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, LUCENE-818.take3.patch, LUCENE-818.take4.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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org