Fix deadlocks in SubIndex shutdown

Because WorkQueue.Module and LuceneIndexModule are in the same
Injector, the ordering of their LifecycleListeners' stop() methods is
not defined. Tasks on the queue, e.g. background mergeability checks,
might try to perform index writes after the SubIndex is shut down.

Shutting down the SubIndex stops the ControlledRealTimeReopenThread,
which immediately sets the generation to Long.MAX_VALUE and notifies
any callers currently waiting on a generation. But the call sites
using NrtFutures in LuceneChangeIndex are not calling NrtFuture.get,
which waits on a generation; they are using Futures.allAsList, which
depends on addListener.

So, we need to check the generation number in the call to addListener
as well as in in get.

This may also improve performance, since futures in some cases will
not need to wait for a reopen thread refresh cycle before notifying
their listeners.

Change-Id: I0ec3e315638a90dd5da3cf7f0d2cc50599a5bb36

1 files changed, 17 insertions, 1 deletions

diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
index 4233a8fe4b..3fe93cfcf7 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
@@ -133,6 +133,20 @@ class SubIndex {
 
   void close() {
     reopenThread.close();
+
+    // Closing the reopen thread sets its generation to Long.MAX_VALUE, but we
+    // still need to refresh the searcher manager to let pending NrtFutures
+    // know.
+    //
+    // Any futures created after this method (which may happen due to undefined
+    // shutdown ordering behavior) will finish immediately, even though they may
+    // not have flushed.
+    try {
+      searcherManager.maybeRefreshBlocking();
+    } catch (IOException e) {
+      log.warn("error finishing pending Lucene writes", e);
+    }
+
     try {
       writer.getIndexWriter().commit();
       writer.getIndexWriter().close(true);
@@ -213,7 +227,9 @@ class SubIndex {
 
     @Override
     public void addListener(Runnable listener, Executor executor) {
-      if (!isDone()) {
+      if (isGenAvailableNowForCurrentSearcher() && !isCancelled()) {
+        set(null);
+      } else if (!isDone()) {
         notDoneNrtFutures.add(this);
       }
       super.addListener(listener, executor);