commit 18ae9168c0204a936427c2a2d25540636aead4da
author Dave Borowitz <dborowitz@google.com> Wed Dec 16 13:58:35 2015 -0500
committer Dave Borowitz <dborowitz@google.com> Thu Dec 17 13:25:02 2015 -0500
tree 094951818d3fb800b3d667282157e14d8891d726
parent 679574697b0eef33ac51b508488e1d6ad3ca90e4

Fix race in Lucene shutdown

After a ControlledRealTimeReopenThread is stopped, any NrtFutures
depending on that thread will never finish; get() will hang forever.
This means if there are any indexing operations happening in the
WorkQueue after indexes are closed, they will never complete, causing
a hang during WorkQueue#stop().

Swap the order that lifecycle listeners are registered between the
index module and the work queue, so that the work queue is shut down
first, and the reopen threads don't shut down and leave stranded
tasks.

Change-Id: I005315336178234a974cde0ac2e51f1e04fbd999

gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java

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 bb69533bf..ac43363 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
@@ -243,6 +243,9 @@
       } else if (isGenAvailableNowForCurrentSearcher()) {
         set(null);
         return true;
+      } else if (!reopenThread.isAlive()) {
+        setException(new IllegalStateException("NRT thread is dead"));
+        return true;
       }
       return false;
     }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java

diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
index 2815099..5108315 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
@@ -333,6 +333,11 @@
     modules.add(SchemaVersionCheck.module());
     modules.add(new DropWizardMetricMaker.RestModule());
     modules.add(new LogFileCompressor.Module());
+
+    // Index module shutdown must happen before work queue shutdown, otherwise
+    // work queue can get stuck waiting on index futures that will never return.
+    modules.add(createIndexModule());
+
     modules.add(new WorkQueue.Module());
     modules.add(new ChangeHookRunner.Module());
     modules.add(new ReceiveCommitsExecutorModule());
@@ -351,7 +356,6 @@
     modules.add(new PluginRestApiModule());
     modules.add(new RestCacheAdminModule());
     modules.add(new GpgModule(config));
-    modules.add(createIndexModule());
     if (MoreObjects.firstNonNull(httpd, true)) {
       modules.add(new CanonicalWebUrlModule() {
         @Override

gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java

diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
index a2947af..48f7767 100644
--- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
+++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
@@ -292,7 +292,6 @@
   private Injector createSysInjector() {
     final List<Module> modules = new ArrayList<>();
     modules.add(new DropWizardMetricMaker.RestModule());
-    modules.add(new WorkQueue.Module());
     modules.add(new ChangeHookRunner.Module());
     modules.add(new ReceiveCommitsExecutorModule());
     modules.add(new DiffExecutorModule());
@@ -306,13 +305,12 @@
     modules.add(new PluginRestApiModule());
     modules.add(new RestCacheAdminModule());
     modules.add(new GpgModule(config));
-    switch (indexType) {
-      case LUCENE:
-        modules.add(new LuceneIndexModule());
-        break;
-      default:
-        throw new IllegalStateException("unsupported index.type = " + indexType);
-    }
+
+    // Index module shutdown must happen before work queue shutdown, otherwise
+    // work queue can get stuck waiting on index futures that will never return.
+    modules.add(createIndexModule());
+
+    modules.add(new WorkQueue.Module());
     modules.add(new CanonicalWebUrlModule() {
       @Override
       protected Class<? extends Provider<String>> provider() {
@@ -332,6 +330,15 @@
     return cfgInjector.createChildInjector(modules);
   }
 
+  private Module createIndexModule() {
+    switch (indexType) {
+      case LUCENE:
+        return new LuceneIndexModule();
+      default:
+        throw new IllegalStateException("unsupported index.type = " + indexType);
+    }
+  }
+
   private void initIndexType() {
     indexType = IndexModule.getIndexType(cfgInjector);
   }