diff --git a/dumpsearch/src/util/dump/SearchIndex.java b/dumpsearch/src/util/dump/SearchIndex.java index a3f848e..d7a47f9 100644 --- a/dumpsearch/src/util/dump/SearchIndex.java +++ b/dumpsearch/src/util/dump/SearchIndex.java @@ -39,6 +39,7 @@ import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.Directory; import org.apache.lucene.store.FSDirectory; +import org.apache.lucene.util.IOUtils; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; import org.slf4j.Logger; @@ -109,6 +110,9 @@ public static SearchIndexBuilder with( @NonNull Dump dump, @NonNull St private FacetsConfig _facetsConfig; private DirectoryTaxonomyReader _taxoReader; private int _version; + private Directory _writerDir; + private Directory _taxoWriterDir; + private Directory _taxoReaderDir; private SearchIndex( @NonNull Dump dump, @NonNull FieldAccessor idFieldAccessor, @NonNull BiConsumer documentBuilder, @NonNull IndexWriterConfig config, @NonNull QueryParser queryParser, @NonNull FacetsConfig facetsConfig, int version ) { @@ -130,18 +134,9 @@ private SearchIndex( @NonNull Dump dump, @NonNull FieldAccessor idFieldAccess public void close() throws IOException { commit(); - if ( _writer != null ) { - _writer.close(); - } - if ( _taxoWriter != null ) { - _taxoWriter.close(); - } - if ( _searcherManager != null ) { - _searcherManager.close(); - } - if ( _taxoReader != null ) { - _taxoReader.close(); - } + // readers must be closed before the writers they were opened on, and the Directory instances + // must be closed last, once nothing still has open files on them + IOUtils.close(_searcherManager, _taxoReader, _writer, _taxoWriter, _writerDir, _taxoWriterDir, _taxoReaderDir); super.close(); } @@ -379,13 +374,13 @@ protected void initLookupMap() { @Override protected void initLookupOutputStream() { try { - Directory facetDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath()); - _taxoWriter = new DirectoryTaxonomyWriter(facetDir); + _taxoWriterDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath()); + _taxoWriter = new DirectoryTaxonomyWriter(_taxoWriterDir); _taxoWriter.commit(); - Directory dir = FSDirectory.open(getLookupFile().toPath()); + _writerDir = FSDirectory.open(getLookupFile().toPath()); _config.setOpenMode(OpenMode.CREATE_OR_APPEND); - _writer = new IndexWriter(dir, _config); + _writer = new IndexWriter(_writerDir, _config); _writer.commit(); } @@ -408,8 +403,8 @@ protected void openSearcher() { } try { - Directory facetDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath()); - _taxoReader = new DirectoryTaxonomyReader(facetDir); + _taxoReaderDir = FSDirectory.open(new File(getLookupFile().getAbsolutePath() + "-facets").toPath()); + _taxoReader = new DirectoryTaxonomyReader(_taxoReaderDir); } catch ( IOException e ) { throw new RuntimeException("Failed to initialize dump facet index with lookup file " + getLookupFile() + "-facets", e); diff --git a/dumpsearch/test/util/dump/SearchIndexTest.java b/dumpsearch/test/util/dump/SearchIndexTest.java index f0f2db4..6c1d63a 100644 --- a/dumpsearch/test/util/dump/SearchIndexTest.java +++ b/dumpsearch/test/util/dump/SearchIndexTest.java @@ -1,6 +1,7 @@ package util.dump; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static util.dump.SearchIndex.SortBuilder.Direction.ASC; import static util.dump.SearchIndex.SortBuilder.Direction.DESC; import static util.dump.SearchIndex.sort; @@ -8,6 +9,7 @@ import java.io.File; import java.io.IOException; +import java.lang.reflect.Field; import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; @@ -18,6 +20,8 @@ import org.apache.lucene.document.TextField; import org.apache.lucene.facet.FacetField; import org.apache.lucene.facet.FacetResult; +import org.apache.lucene.store.AlreadyClosedException; +import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; import org.junit.After; import org.junit.Before; @@ -165,6 +169,30 @@ public void testFaceting() throws Exception { } } + /** + * Regression test for the leaked {@link Directory} handles that used to keep the Lucene index directory locked + * and writable after {@link SearchIndex#close()} returned, which surfaced as flaky {@code DirectoryNotEmptyException}s + * in callers deleting the underlying folder right after close (e.g. JUnit 5's {@code @TempDir}). + */ + @Test + public void testCloseReleasesUnderlyingLuceneDirectories() throws Exception { + File dumpFile = new File(_tmpdir, DUMP_FILENAME); + SearchIndex index; + + try (Dump dump = new Dump<>(Bean.class, dumpFile)) { + index = with(dump, "_idLong", ( doc, o ) -> doc.add(new FacetField("facetField", o._data))).build(); + dump.add(new Bean(1, "row")); + } // closes dump, which cascades to index.close() + + for ( String fieldName : new String[]{ "_writerDir", "_taxoWriterDir", "_taxoReaderDir" } ) { + Field field = SearchIndex.class.getDeclaredField(fieldName); + field.setAccessible(true); + Directory dir = (Directory)field.get(index); + assertThat(dir).as(fieldName + " was never opened").isNotNull(); + assertThatThrownBy(dir::listAll).as(fieldName + " was not closed by SearchIndex.close()").isInstanceOf(AlreadyClosedException.class); + } + } + @Test public void testVersion() throws Exception { File dumpFile = new File(_tmpdir, DUMP_FILENAME); diff --git a/pom.xml b/pom.xml index b1dcd71..5772b59 100644 --- a/pom.xml +++ b/pom.xml @@ -23,10 +23,10 @@ maven-compiler-plugin + 3.15.0 UTF-8 - 17 - 17 + 17