[
https://issues.apache.org/jira/browse/LUCENE-6113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14248848#comment-14248848
]
ryan rawson commented on LUCENE-6113:
-------------------------------------
most people should be seeing the NPE because assertions are not always enabled.
The bigger question to me is API design. as it stands, people MUST guard their
call in to this method call since there is no way to ensure that it will always
be not-null (since the place you get one may fail).
As an illustrative example, in the Cocoa API, the 'release' refcount calls will
do nothing to a null reference for similar reasons (dont spread null checking
everywhere else in the code).
So finally, what's the code scenario whereby a 'reader ref count leak' could be
made by doing the ignore a null release? The app code already has to make that
if guard, so why not absorb it in to the API?
> 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: [email protected]
For additional commands, e-mail: [email protected]
