I'm afraid that always calling doClose would be problematic. If the reader is used in another thread then files might get closed under the hood while a search is being performed. Even if the guard that we added to MMapDirectory would now prevent segfaults, users would get potentially confusing exception.
I see pros and cons regarding checking "if (closed)" in ensureOpen. In any case I think we should keep a way to let ongoing searches finish successfully before effectively closing the reader, which I suspect users rely on. Le mar. 27 mars 2018 à 15:19, Dawid Weiss <[email protected]> a écrit : > I'm looking at IndexReader.close and keep wondering: > > /** > * Closes files associated with this index. > * Also saves any new deletions to disk. > * No other methods should be called after this has been called. > * @throws IOException if there is a low-level IO error > */ > @Override > public final synchronized void close() throws IOException { > if (!closed) { > decRef(); > closed = true; > } > } > > If you have refCount > 1 this leads to an odd scenario in which the > decRef decreases the counter, but does not call doClose. It also sets > 'closed' > to true, but the object remains functional (since ensureClosed checks > refCount only). > > Wouldn't it be better for close() to actually check the refCount and > (if closed == false): > > - always call doClose > - mark the object as closed (closed = true) > - if refCount != 1 => unpaired incRef/decRef somewhere, signal an > exception? > > Then the javadoc would stay true, even if somebody played with reference > counts. > > Dawid > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
