I think it's an awful bug to mask! Screwing up refCounts is very bad and if your app is sometimes calling .close and other times calling .decRef you are in serious trouble.
Mike McCandless http://blog.mikemccandless.com On Wed, Oct 5, 2011 at 9:03 AM, Shai Erera <[email protected]> wrote: > 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] >> > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
