You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Robert Muir (JIRA)" <ji...@apache.org> on 2011/09/15 21:57:09 UTC
[jira] [Created] (LUCENE-3439) add checks/asserts if you search
across a closed reader
add checks/asserts if you search across a closed reader
-------------------------------------------------------
Key: LUCENE-3439
URL: https://issues.apache.org/jira/browse/LUCENE-3439
Project: Lucene - Java
Issue Type: Bug
Reporter: Robert Muir
if you try to search across a closed reader (and/or searcher too),
there are no checks, not even assertions statements.
this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Robert Muir (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13105665#comment-13105665 ]
Robert Muir commented on LUCENE-3439:
-------------------------------------
{quote}
IR/IW already try to do this
{quote}
I don't think so? I'll attach a patch with a trivial test basically demonstrating what I was seeing.
It sometimes passes!!!!!
{noformat}
public void test() throws Exception {
TermRangeQuery query = TermRangeQuery.newStringRange("field", "a", "z", true, true);
searcher.search(query, 5);
searcher.close();
reader.close();
searcher.search(query, 5); // are you serious?
}
{noformat}
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13106738#comment-13106738 ]
Michael McCandless commented on LUCENE-3439:
--------------------------------------------
bq. and we shouldn't penalize correct programs
I agree we should keep the added cost for correct programs to a
minimum, but there are many good reasons why we should try to throw
AlreadyClosedException instead of something scary like NPE or
EOFException, etc.:
* Better usability -- when you make a simple mistake (IW/IR.close in
the wrong place) you get a clear indication what's wrong, and that
saves you time/frustration/hair/iterations.
* Earlier error detection: lower risk that an app will push to
production without catching that IW/IR.close is wrong.
* Save Lucene devs time: when users come to the list w/ a cryptic
exception that after a few back & forths we discover was an errant
IW/IR.close, that uses up lots of people's time, time that instead
could be spent towards improving Lucene/Solr instead.
* Protect Lucene's perceived quality: like it or not, Lucene (not
the app) will sometimes be blamed when a user hits a cryptic
exception because of an errant IW/IR.close. Blog posts will go
up, tweets will get tweeted, emails to list or to other lists will
go unanswered, people doing future Googling will hit these and
see Lucene as buggy.
* We can find our own bugs -- in adding some missing ensureOpens to
IW, here, I discovered a case where DirectoryReader.close calls
IW.deleteUnusedFiles after IW was closed, and this invokes the
DelPolicy, which is definitely dangerous.
Net/net I think it's important that we try, when possible/reasonable,
to have clear error detection and reporting, making it as easy as we
can for the user to understand what's wrong. And if that means a
miniscule extra cost to "correct" users, that's a fair tradeoff.
bq. While a volatile read on (current) x86 is a no-op, it still imposes a happens-before/after restriction via the memory model and hence prevents optimizations across that volatile read. Volatile reads may not always be a no-op on x86 either (and they already aren't on other CPUs).
The thing is, we already call ensureOpen only in places where the
added cost should be vanishingly small compared to what will follow;
it'll still be vanishingly small as a volatile.
Actually, IR's ensureOpen checks the refCount (AtomicLong)
so we are already "effectively" volatile for IR, just not for IW.
bq. Using a volatile also doesn't buy us much either - it's still best-effort (the close could come after we have checked the volatile, but before we have done the read).
Right this will always be a best effort check, but if we can do a
"better" best effort, while still having no real added cost, I think
we should improve.
I'm not saying we should add ensureOpen() in hotspots. But I do think
we should make IW's closed boolean volatile. The added cost will
still be tiny, since we only call ensureOpen where it shouldn't
matter, and it may allow us to throw ACE instead of a cryptic
exception in some cases.
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
> Attachments: LUCENE-3439_test.patch
>
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13105685#comment-13105685 ]
Yonik Seeley commented on LUCENE-3439:
--------------------------------------
I think the open/closed checks should still be non-volatile.
While a volatile read on (current) x86 is a no-op, it still imposes a happens-before/after restriction via the memory model and hence prevents optimizations across that volatile read. Volatile reads may not always be a no-op on x86 either (and they already aren't on other CPUs).
Using a volatile also doesn't buy us much either - it's still best-effort (the close could come after we have checked the volatile, but before we have done the read). We have enough volatile/synchronized reads in other places that a non-volatile check will suffice to catch bugs (and we shouldn't penalize correct programs).
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
> Attachments: LUCENE-3439_test.patch
>
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Commented] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13105663#comment-13105663 ]
Michael McCandless commented on LUCENE-3439:
--------------------------------------------
I think we should add real checks here, w/ volatile isOpen/isClosed bool, up and down the stack, to those methods where the volatile read cost will be tiny compared to the cost of the method or of the app using the returned result.
IR/IW already try to do this (hmm: though not volatile for IW) but it sounds like we are missing checks inside the codecs?
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Updated] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Robert Muir (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Robert Muir updated LUCENE-3439:
--------------------------------
Attachment: LUCENE-3439_test.patch
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
> Attachments: LUCENE-3439_test.patch
>
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Updated] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless updated LUCENE-3439:
---------------------------------------
Attachment: LUCENE-3439.patch
Patch, adding missing ensureOpens, adding volatile to IW's close/closing bools, and fixing the scary bug where we invoke del policy (and possibly delete file) on a closed writer.
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
> Attachments: LUCENE-3439.patch, LUCENE-3439_test.patch
>
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Resolved] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless resolved LUCENE-3439.
----------------------------------------
Resolution: Fixed
Fix Version/s: 4.0
3.5
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
> Assignee: Michael McCandless
> Fix For: 3.5, 4.0
>
> Attachments: LUCENE-3439.patch, LUCENE-3439_test.patch
>
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[jira] [Assigned] (LUCENE-3439) add checks/asserts if you search
across a closed reader
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-3439?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless reassigned LUCENE-3439:
------------------------------------------
Assignee: Michael McCandless
> add checks/asserts if you search across a closed reader
> -------------------------------------------------------
>
> Key: LUCENE-3439
> URL: https://issues.apache.org/jira/browse/LUCENE-3439
> Project: Lucene - Java
> Issue Type: Bug
> Reporter: Robert Muir
> Assignee: Michael McCandless
> Attachments: LUCENE-3439.patch, LUCENE-3439_test.patch
>
>
> if you try to search across a closed reader (and/or searcher too),
> there are no checks, not even assertions statements.
> this results in crazy scary stacktraces deep inside places like FSTs/various term dictionary implementations etc.
> In some situations, depending on codec, you wont even get an error (i'm sure its fun when you try to retrieve the stored fields!)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org