I think it's a harmless bug to mask, but ok. Shai
On Wed, Oct 5, 2011 at 2:41 PM, Michael McCandless < [email protected]> wrote: > You mean when we call doClose inside IR.decRef should we set close=true? > > I'm not sure we should... close will only be false at that point if > the consumer who opened the IR called .decRef instead of .close to > drop the reference. This is actually a fine thing to do, but, if that > same consumer later calls close, today they will get an exception (too > many decRefs) and I think we should keep that exception since it tells > you your app has a bug? > > (Vs if we did set closed=true we'd mask that exception). > > Mike McCandless > > http://blog.mikemccandless.com > > On Wed, Oct 5, 2011 at 7:11 AM, Shai Erera <[email protected]> wrote: > > Should decRef perhaps set closed=true? Otherwise, calling close() would > > result in an IllegalStateException, which is not very useful or necessary > I > > think. > > > > Shai > > > > On Wed, Oct 5, 2011 at 12:34 PM, Michael McCandless > > <[email protected]> wrote: > >> > >> Right, when you call IR.close() what that translates to is "drop the > >> ref that had been returned when this IR was opened". > >> > >> Mike McCandless > >> > >> http://blog.mikemccandless.com > >> > >> On Wed, Oct 5, 2011 at 5:20 AM, Uwe Schindler <[email protected]> wrote: > >> > Hi Shai, > >> > > >> > > >> > > >> > We implement java.io.Closeable and the semantics in the interface > >> > Closeable > >> > require that you can call close() multiple times without effect: > >> > > >> > http://download.oracle.com/javase/6/docs/api/java/io/Closeable.html > >> > > >> > > >> > > >> > “Closes this stream and releases any system resources associated with > >> > it. If > >> > the stream is already closed then invoking this method has no effect.” > >> > > >> > > >> > > >> > ----- > >> > > >> > Uwe Schindler > >> > > >> > H.-H.-Meier-Allee 63, D-28213 Bremen > >> > > >> > http://www.thetaphi.de > >> > > >> > eMail: [email protected] > >> > > >> > > >> > > >> > From: Shai Erera [mailto:[email protected]] > >> > Sent: Wednesday, October 05, 2011 11:17 AM > >> > To: [email protected]; [email protected] > >> > Subject: Re: IndexReader.close > >> > > >> > > >> > > >> > After I made the change, TestIndexReaderReopen failed. Reason is that > it > >> > is > >> > assumed that one can call close() many times, without it affecting the > >> > IndexReader instance. However, if I change the call to use refCount > >> > instead > >> > of closes, then calling close() multiple times decreases the ref count > >> > multiple times, until it is 0. > >> > > >> > So I think we better keep closed, just in case someone out there > relies > >> > on > >> > this behavior. > >> > > >> > Shai > >> > > >> > On Wed, Oct 5, 2011 at 9:35 AM, Simon Willnauer > >> > <[email protected]> wrote: > >> > > >> > +1 > >> > > >> > On Wed, Oct 5, 2011 at 6:26 AM, Shai Erera <[email protected]> wrote: > >> >> Hi > >> >> > >> >> I noticed that IndexReader.close() sets a 'close' member to true, but > >> >> we > >> >> never check this member. Can we eliminate it and change close() to > >> >> check > >> >> refCount.get() > 0? > >> >> > >> >> Shai > >> >> > >> > > >> > --------------------------------------------------------------------- > >> > To unsubscribe, e-mail: [email protected] > >> > For additional commands, e-mail: [email protected] > >> > > >> > > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [email protected] > >> For additional commands, e-mail: [email protected] > >> > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
