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

Shai Erera commented on LUCENE-5553:
------------------------------------

bq. if the last IndexReader#decRef() call runs into an exception on 
IndexReader#doClose(). Today we just reset the refCount and never go and call 
the listeners.

As I read the code, if doClose() hits an exception, we put the reference back, 
then the exception is thrown further and therefore {{closed}} isn't set to 
true. So the reader is in fact not closed, and therefore you can attempt to 
{{r.close()}} is again? And also that's probably why we don't notify the 
listeners?

bq.  I think we need to fix the loop in 
IndexReader#notifyReaderClosedListeners() to make sure we call all of them

+1!

bq. It also seems that SegmentCoreReaders#decRef() has a similar problem where 
for instance a fieldsReader can throw an exception on close and we never call 
the core listeners.

Hmm ... this is trickier. Here, unlike IndexReader.close(), we don't put the 
reference back. So if that's a bug, we should put back the reference if any of 
the IOUtils.close() failed. But if that's ok, then +1 to notify the listeners 
irregardless if close hit an exception.






> IndexReader#ReaderClosedListener is not always called on IndexReader#close()
> ----------------------------------------------------------------------------
>
>                 Key: LUCENE-5553
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5553
>             Project: Lucene - Core
>          Issue Type: Bug
>          Components: core/index
>    Affects Versions: 4.7, 5.0
>            Reporter: Simon Willnauer
>            Priority: Blocker
>             Fix For: 4.7.1
>
>
> Today IndexReader#ReaderClosedListener might not be called if the last 
> IndexReader#decRef() call runs into an exception on IndexReader#doClose(). 
> Today we just reset the refCount and never go and call the listeners. There 
> seem to be a bunch of problems here along the same lines but IMO if we close 
> a reader it should close all resources no matter what exception it runs into. 
> What this should do is call the close listeners in a finally block and then 
> rethrow the exception. The real problem here for apps relying on the listener 
> to release resources is that you might leak memory or file handles or whatnot 
> which I think is a bug how we handle closing the IR. As a side-note I think 
> we should never reset the reference here to be honest.  
> Along the same lines I think we need to fix the loop in 
> IndexReader#notifyReaderClosedListeners() to make sure we call all of them in 
> the case any of them throws an exception. It also seems that 
> SegmentCoreReaders#decRef() has a similar problem where for instance a 
> fieldsReader can throw an exception on close and we never call the core 
> listeners.
> IMO we need to fix this for 4.7.1 



--
This message was sent by Atlassian JIRA
(v6.2#6252)

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

Reply via email to