test: regression test for the legacy index drop race (#1315) (#1319)

Author: anidotnet

CHANGELOG.md

@@ -27,10 +27,11 @@ Eleven changes from [@brettwooldridge](https://github.com/brettwooldridge), most
 - **`clear()` and `dropIndex()` reach every layout map an index occupies** ([#1295](https://github.com/nitrite/nitrite-java/pull/1295))
   - `IndexManager.close()`, `clearAll()` and `dropIndexDescriptor()` acted only on the map name recorded in `IndexMeta`, which is the classic one. That already missed the composite map of a non-unique index: after `collection.clear()` its rows survived, and a query on that index returned the ids of the cleared documents alongside the new ones - two live documents, four results. It also broke `ChangeIdField`, whose `createIndex` found the previous index map still populated and rebuilt over it.
 
-- **The first concurrent read of an index after a restart no longer fails dropping its legacy map** ([#1309](https://github.com/nitrite/nitrite-java/pull/1309))
+- **The first concurrent read of an index after a restart no longer fails dropping its legacy map** ([#1309](https://github.com/nitrite/nitrite-java/pull/1309), [#1315](https://github.com/nitrite/nitrite-java/issues/1315))
   - The first read of an index still in a legacy layout migrates it and drops the legacy map, once per index instance. `ComparableIndexer` created those instances with an unsynchronized check-then-act, so threads arriving together could each get an instance of their own and each run the migration. On MVStore the second drop asked `MVMap.getName()` for a map that was already gone, got `null`, and failed with `NullPointerException` in `NitriteMVStore.removeMap`; the same race also threw from `Attributes.set` through `NitriteMap.updateLastModifiedTime`.
   - Observed on a production system on the first multi-threaded lookup after every restart, because every close before [#1295](https://github.com/nitrite/nitrite-java/pull/1295) left an empty map under the legacy name for the next start to drop.
   - The indexer and the MVStore map and R-tree registries now create their entries with `computeIfAbsent`, so there is one index instance and one map wrapper per name. `NitriteMVMap` keeps the name it was opened with and acts only when its compare-and-set wins, so `drop()` and `close()` run once; `removeMap` ignores a null name and no longer creates an empty map only to remove it.
+  - Reported against 5.3.0 in [#1315](https://github.com/nitrite/nitrite-java/issues/1315): four threads making the first find on a reopened file database failed 21 of 400 finds on 5.3.0 and none with these changes. The reporter's reproduction is now `Issue1315Test`.
 
 - **Concurrent first reads of a map on an in-memory MVStore no longer fail inside H2** ([#1311](https://github.com/nitrite/nitrite-java/pull/1311))
   - H2's `ObjectDataType` picks the delegate that compares serialized keys on first use, through an unsynchronized field, and `SerializedObjectType.compare` checks delegates by identity. Threads making a map's first key comparison together could each install their own and fail with `UnsupportedOperationException: Can not compare`. The code is the same in h2 2.4.240 and 2.5.250.

nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/collection/Issue1315Test.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2017-2021 Nitrite author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.dizitart.no2.integration.collection;
+
+import org.dizitart.no2.Nitrite;
+import org.dizitart.no2.collection.Document;
+import org.dizitart.no2.collection.NitriteCollection;
+import org.dizitart.no2.filters.FluentFilter;
+import org.dizitart.no2.index.IndexOptions;
+import org.dizitart.no2.index.IndexType;
+import org.dizitart.no2.mvstore.MVStoreModule;
+import org.junit.Test;
+
+import java.io.File;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CyclicBarrier;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Regression test for <a href="https://github.com/nitrite/nitrite-java/issues/1315">Issue 1315</a>.
+ * <p>
+ * On 5.3.0 every close left an empty legacy index map behind, and the first finds on a
+ * reopened file database raced to migrate and drop it, failing with a
+ * {@code NullPointerException} in {@code NitriteMVStore.removeMap}. Fixed by #1295 and #1309.
+ */
+public class Issue1315Test {
+
+    private static Nitrite open(File file) {
+        return Nitrite.builder()
+            .loadModule(MVStoreModule.withConfig().filePath(file.getPath()).build())
+            .openOrCreate();
+    }
+
+    @Test
+    public void firstUseOfANonUniqueIndexFromSeveralThreads() throws Exception {
+        File file = File.createTempFile("nitrite-legacy-race", ".db");
+        file.delete();
+        try {
+            Nitrite db = open(file);
+            NitriteCollection items = db.getCollection("items");
+            items.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "state");
+            items.insert(Document.createDocument("state", "new"));
+            db.close();
+
+            int threads = 4, openings = 100;
+            List<Throwable> failures = new CopyOnWriteArrayList<>();
+            for (int opening = 0; opening < openings; opening++) {
+                Nitrite reopened = open(file);
+                NitriteCollection collection = reopened.getCollection("items");
+                CyclicBarrier start = new CyclicBarrier(threads);
+                Thread[] workers = new Thread[threads];
+                for (int i = 0; i < threads; i++) {
+                    workers[i] = new Thread(() -> {
+                        try {
+                            start.await();
+                            assertEquals(1, collection.find(FluentFilter.where("state").eq("new")).toList().size());
+                        } catch (Throwable t) {
+                            failures.add(t);
+                        }
+                    });
+                    workers[i].start();
+                }
+                for (Thread worker : workers) worker.join();
+                reopened.close();
+            }
+            assertEquals(failures.toString(), 0, failures.size());
+        } finally {
+            file.delete();
+        }
+    }
+}