This is the code in IndexReader.close():
public final synchronized void close() throws IOException {
if (!closed) {
decRef();
closed = true;
}
}
What strikes me as odd is that “closed” variable is set to true regardless of
whether the index was actually closed using doClose(). It is causing a bit of a
problem in the following setup.
We have many indexes. Some of these indexes are kept open and IndexReader is
reused for performance reasons, some are open just for a single search. Some of
them are combined together at the search time depending on the user performing
the search. For every search, if there are multiple relevant IndexReader, we
create a new MultiReader instance and close it once the search is done. Since
reference counting is already built into the IndexReader, I figured I can just
call incRef() when using a cached IndexReader and then call close() or let
MultiReader call close() on it once it is done. The problem is only the first
close() calls decRef(),subsequent calls do nothing, and if there were multiple
incRef() calls, the IndexReader ends up being left open.
I thought of using decRef() instead of close() but MultiReader doesn’t have an
option of calling decRef() on subreaders, only close().
The following change to IndexReader.close() fixes the problem:
public final synchronized void close() throws IOException {
if (refCount.get() > 0) {
decRef();
}
}
It seems more logical. Also it is now consistent with ensureOpen() method.
Am I missing something here? Is there a good reason to prevent close() from
doing anything after the first call even if the IndexReader was not actually
closed? Does it even need to be synchronized if decRef() does all the work?
Thanks,
Alexey
