SOLR-4400: Deadlock can occur in a rare race between committing and closing a SolrIndexWriter.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1444152 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -108,6 +108,9 @@ Bug Fixes
 * SOLR-4380: Replicate after startup option would not replicate until the
   IndexWriter was lazily opened. (Mark Miller, Gregg Donovan)
 
+* SOLR-4400: Deadlock can occur in a rare race between committing and
+  closing a SolrIndexWriter. (Erick Erickson, Mark Miller)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java

@@ -18,6 +18,8 @@ package org.apache.solr.update;
  */
 
 import java.io.IOException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.store.AlreadyClosedException;
@@ -52,6 +54,8 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover
   private boolean pauseWriter;
   private boolean writerFree = true;
   
+  protected final ReentrantLock commitLock = new ReentrantLock();
+
   public DefaultSolrCoreState(DirectoryFactory directoryFactory) {
     this.directoryFactory = directoryFactory;
   }
@@ -135,13 +139,26 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover
       pauseWriter = true;
       // then lets wait until its out of use
       log.info("Waiting until IndexWriter is unused... core=" + coreName);
-      while (!writerFree) {
-        try {
-          writerPauseLock.wait(100);
-        } catch (InterruptedException e) {}
+      
+      boolean yieldedCommitLock = false;
+      try {
+        if (commitLock.isHeldByCurrentThread()) {
+          yieldedCommitLock = true;
+          commitLock.unlock();
+        }
         
-        if (closed) {
-          throw new RuntimeException("SolrCoreState already closed");
+        while (!writerFree) {
+          try {
+            writerPauseLock.wait(100);
+          } catch (InterruptedException e) {}
+          
+          if (closed) {
+            throw new RuntimeException("SolrCoreState already closed");
+          }
+        }
+      } finally {
+        if (yieldedCommitLock) {
+          commitLock.lock();
         }
       }
 
@@ -272,4 +289,9 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover
     closeIndexWriter(closer);
   }
   
+  @Override
+  public Lock getCommitLock() {
+    return commitLock;
+  }
+  
 }

solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java

@@ -29,8 +29,6 @@ import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.lucene.document.Document;
 import org.apache.lucene.index.IndexReader;
@@ -70,7 +68,6 @@ import org.apache.solr.util.RefCounted;
  */
 public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState.IndexWriterCloser {
   protected final SolrCoreState solrCoreState;
-  protected final Lock commitLock = new ReentrantLock();
 
   // stats
   AtomicLong addCommands = new AtomicLong();
@@ -502,7 +499,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
     try {
       // only allow one hard commit to proceed at once
       if (!cmd.softCommit) {
-        commitLock.lock();
+        solrCoreState.getCommitLock().lock();
       }
 
       log.info("start "+cmd);
@@ -596,7 +593,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
     }
     finally {
       if (!cmd.softCommit) {
-        commitLock.unlock();
+        solrCoreState.getCommitLock().unlock();
       }
 
       addCommands.set(0);
@@ -680,7 +677,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
   @Override
   public void closeWriter(IndexWriter writer) throws IOException {
     boolean clearRequestInfo = false;
-    commitLock.lock();
+    solrCoreState.getCommitLock().lock();
     try {
       SolrQueryRequest req = new LocalSolrQueryRequest(core, new ModifiableSolrParams());
       SolrQueryResponse rsp = new SolrQueryResponse();
@@ -745,7 +742,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
       if (writer != null) writer.close();
 
     } finally {
-      commitLock.unlock();
+      solrCoreState.getCommitLock().unlock();
       if (clearRequestInfo) SolrRequestInfo.clearRequestInfo();
     }
   }

solr/core/src/java/org/apache/solr/update/SolrCoreState.java

@@ -18,6 +18,7 @@ package org.apache.solr.update;
  */
 
 import java.io.IOException;
+import java.util.concurrent.locks.Lock;
 
 import org.apache.lucene.index.IndexWriter;
 import org.apache.solr.core.CoreContainer;
@@ -37,6 +38,8 @@ public abstract class SolrCoreState {
     return deleteLock;
   }
   
+  public abstract Lock getCommitLock();
+  
   /**
    * Force the creation of a new IndexWriter using the settings from the given
    * SolrCore.