Always hold a reference to the Executorservice

apache · Feb 21, 2024 · 2740318 · 2740318
1 parent c054414
commit 2740318
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 2 deletions.
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -117,7 +117,6 @@ public class Http2SolrClient extends Http2SolrClientBase {
   private final HttpClient httpClient;
 
   private SSLConfig sslConfig;
-  ;
 
   private List<HttpListenerFactory> listenerFactory = new ArrayList<>();
   private final AsyncTracker asyncTracker = new AsyncTracker();

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrJdkClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrJdkClient.java
@@ -35,6 +35,8 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ExecutorService;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -60,6 +62,10 @@ public class HttpSolrJdkClient extends Http2SolrClientBase {
 
   private HttpClient client;
 
+  protected ExecutorService executor;
+
+  private boolean shutdownExecutor;
+
   protected HttpSolrJdkClient(String serverBaseUrl, HttpSolrJdkClient.Builder builder) {
     super(serverBaseUrl, builder);
 
@@ -71,9 +77,16 @@ protected HttpSolrJdkClient(String serverBaseUrl, HttpSolrJdkClient.Builder buil
     if (builder.sslContext != null) {
       b.sslContext(builder.sslContext);
     }
+
     if (builder.executor != null) {
-      b.executor(builder.executor);
+      this.executor = builder.executor;
+      this.shutdownExecutor = false;
+    } else {
+      this.executor = Executors.newCachedThreadPool(Executors.defaultThreadFactory());
+      this.shutdownExecutor = true;
     }
+    b.executor(this.executor);
+
     if (builder.proxyHost != null) {
       if (builder.proxyIsSocks4) {
         log.warn(
@@ -266,8 +279,14 @@ private NamedList<Object> processErrorsAndResponse(
 
   @Override
   public void close() throws IOException {
+    if(shutdownExecutor) {
+      executor.shutdown();
+    }
+    executor = null;
+
     // TODO: Java 21 adds close/autoclosable to HttpClient.  We should use it.
     client = null;
+
     assert ObjectReleaseTracker.release(this);
   }
 

diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrJdkClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrJdkClientTest.java
@@ -21,6 +21,8 @@
 import java.net.Socket;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
@@ -408,6 +410,24 @@ public void testContentTypeToEncoding() throws Exception {
     }
   }
 
+  @Test
+  public void testPassedInExecutorNotShutdown() throws Exception {
+    ExecutorService myExecutor = null;
+    try {
+      myExecutor = Executors.newSingleThreadExecutor();
+      try (HttpSolrJdkClient client = builder(getBaseUrl()).withExecutor(myExecutor).build()) {
+        assertEquals(myExecutor, client.executor);
+      }
+      assertFalse(myExecutor.isShutdown());
+    } finally {
+      try {
+        myExecutor.shutdownNow();
+      } catch(Exception e1) {
+        //ignore
+      }
+    }
+  }
+
   @Override
   protected String expectedUserAgent() {
     return "Solr[" + HttpSolrJdkClient.class.getName() + "] 1.0";