[jira] [Commented] (LUCENE-6113) ReferenceManager.release uses assertion to expect argument not null, also expects argument to be not null

["Michael McCandless (JIRA)" <ji...@apache.org>]

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

Michael McCandless commented on LUCENE-6113:
--------------------------------------------

I agree this is a hassle, but I think it'd be dangerous to accept null in release and ignore it?  It could mask a real bug (reader ref count leak) in the application.

We could simple drop the assert: the app will see NPE which should be self explanatory.

> ReferenceManager.release uses assertion to expect argument not null, also expects argument to be not null
> ---------------------------------------------------------------------------------------------------------
>
>                 Key: LUCENE-6113
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6113
>             Project: Lucene - Core
>          Issue Type: Bug
>    Affects Versions: 4.10.1
>            Reporter: ryan rawson
>
> A common use pattern for the Reference Manager looks like so:
> {code}
> IndexSearcher searcher = null;
> try {
>     searcher = searcherManager.acquire();
>     // do real work
> } finally {
>    searcherManager.release(searcher);
> }
> {code}
> The problem with this code is if 'acquire' throws an exception, the finally block is called with a null reference for 'searcher'.  There are two issues, one is this call release() uses assertion to check for argument validity, which is not recommended (http://docs.oracle.com/javase/8/docs/technotes/guides/language/assert.html) and secondly to fix this, we need to guard all calls to release with an if clause.
> Why not have release() be a noop if it is passed null, instead of triggering an NPE?  It would support this API usage pattern w/o any changes on the behalf of users.
> Looking at the code, it appears that it is very unlikely that the acquire() call throws an exception. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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