This is an automated email from the ASF dual-hosted git repository.
janhoy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new dc441bc7beb SOLR-18472 TikaServerExtractionBackend can hand out a
reference to a stopped HttpClient (#4930)
dc441bc7beb is described below
commit dc441bc7bebdff99346c3761b0a5e55904a8bd6d
Author: Jan Høydahl <[email protected]>
AuthorDate: Fri Oct 9 00:11:53 2026 +0200
SOLR-18472 TikaServerExtractionBackend can hand out a reference to a
stopped HttpClient (#4930)
---
.../SOLR-18472-tikaserver-httpclient-refcount.yml | 10 ++++
.../extraction/TikaServerExtractionBackend.java | 39 ++++++-------
.../TikaServerExtractionBackendTest.java | 65 ++++++++++++++++++++++
3 files changed, 93 insertions(+), 21 deletions(-)
diff --git a/changelog/unreleased/SOLR-18472-tikaserver-httpclient-refcount.yml
b/changelog/unreleased/SOLR-18472-tikaserver-httpclient-refcount.yml
new file mode 100644
index 00000000000..5caf01ba1fa
--- /dev/null
+++ b/changelog/unreleased/SOLR-18472-tikaserver-httpclient-refcount.yml
@@ -0,0 +1,10 @@
+title: >
+ Fixed a race in the Tika Server extraction backend where closing or
reloading one core could leave
+ another core using a stopped HTTP client.
+type: fixed
+authors:
+ - name: Jan Høydahl
+ nick: janhoy
+links:
+ - name: SOLR-18472
+ url: https://issues.apache.org/jira/browse/SOLR-18472
diff --git
a/solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java
b/solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java
index a6d08bb11f7..2c422f7ebfe 100644
---
a/solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java
+++
b/solr/modules/extraction/src/java/org/apache/solr/handler/extraction/TikaServerExtractionBackend.java
@@ -33,6 +33,7 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.ExecutorUtil;
import org.apache.solr.common.util.NamedList;
@@ -67,11 +68,13 @@ public class TikaServerExtractionBackend implements
ExtractionBackend {
private HashMap<String, Object> initArgsMap = new HashMap<>();
private final long maxCharsLimit;
- // Singleton holder for the shared HttpClient/Executor resources (one per
JVM)
- private static volatile RefCounted<HttpClientResources> SHARED_RESOURCES;
+ // Singleton holder for the shared HttpClient/Executor resources (one per
JVM), guarded by
+ // INIT_LOCK so that acquiring a reference cannot race with the last
decref() closing it
+ private static RefCounted<HttpClientResources> SHARED_RESOURCES;
// Per-backend handle (same RefCounted instance as SHARED_RESOURCES) that
this instance will
// decref() on close
- private RefCounted<HttpClientResources> acquiredResourcesRef;
+ private final RefCounted<HttpClientResources> acquiredResourcesRef;
+ private final AtomicBoolean closed = new AtomicBoolean();
public TikaServerExtractionBackend(String baseUrl) {
this(baseUrl, DEFAULT_TIMEOUT_SECONDS, null, DEFAULT_MAXCHARS_LIMIT);
@@ -117,7 +120,7 @@ public class TikaServerExtractionBackend implements
ExtractionBackend {
Duration.ofSeconds(timeoutSeconds > 0 ? timeoutSeconds :
DEFAULT_TIMEOUT_SECONDS);
// Acquire a reference to the shared resources; keep a handle so we can
decref() on close
- acquiredResourcesRef = initializeHttpClient().incref();
+ acquiredResourcesRef = acquireSharedResources();
}
public static final String NAME = "tikaserver";
@@ -367,6 +370,7 @@ public class TikaServerExtractionBackend implements
ExtractionBackend {
@Override
protected void close() {
+ assert Thread.holdsLock(INIT_LOCK);
// stop client and shutdown executor
try {
if (resource.client != null) resource.client.stop();
@@ -376,20 +380,16 @@ public class TikaServerExtractionBackend implements
ExtractionBackend {
if (resource.executor != null) resource.executor.shutdownNow();
} catch (Throwable ignore) {
}
- synchronized (INIT_LOCK) {
- // clear the shared reference when closed
- if (SHARED_RESOURCES == this) {
- SHARED_RESOURCES = null;
- }
+ // clear the shared reference when closed
+ if (SHARED_RESOURCES == this) {
+ SHARED_RESOURCES = null;
}
}
}
- private static RefCounted<HttpClientResources> initializeHttpClient() {
- RefCounted<HttpClientResources> ref = SHARED_RESOURCES;
- if (ref != null) return ref;
+ private static RefCounted<HttpClientResources> acquireSharedResources() {
synchronized (INIT_LOCK) {
- if (SHARED_RESOURCES != null) return SHARED_RESOURCES;
+ if (SHARED_RESOURCES != null) return SHARED_RESOURCES.incref();
ThreadFactory tf = new SolrNamedThreadFactory("TikaServerHttpClient");
ExecutorService exec = ExecutorUtil.newMDCAwareCachedThreadPool(tf);
HttpClient client = new HttpClient();
@@ -406,7 +406,7 @@ public class TikaServerExtractionBackend implements
ExtractionBackend {
SolrException.ErrorCode.SERVER_ERROR, "Failed to start shared
Jetty HttpClient", e);
}
SHARED_RESOURCES = new ResourcesRef(new HttpClientResources(client,
exec));
- return SHARED_RESOURCES;
+ return SHARED_RESOURCES.incref();
}
}
@@ -447,13 +447,10 @@ public class TikaServerExtractionBackend implements
ExtractionBackend {
@Override
public void close() {
- RefCounted<HttpClientResources> ref;
- synchronized (INIT_LOCK) {
- ref = acquiredResourcesRef;
- acquiredResourcesRef = null;
- }
- if (ref != null) {
- ref.decref();
+ if (closed.compareAndSet(false, true)) {
+ synchronized (INIT_LOCK) {
+ acquiredResourcesRef.decref();
+ }
}
}
}
diff --git
a/solr/modules/extraction/src/test/org/apache/solr/handler/extraction/TikaServerExtractionBackendTest.java
b/solr/modules/extraction/src/test/org/apache/solr/handler/extraction/TikaServerExtractionBackendTest.java
index 326ab818596..4505a346fe4 100644
---
a/solr/modules/extraction/src/test/org/apache/solr/handler/extraction/TikaServerExtractionBackendTest.java
+++
b/solr/modules/extraction/src/test/org/apache/solr/handler/extraction/TikaServerExtractionBackendTest.java
@@ -24,10 +24,15 @@ import java.nio.file.Files;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import org.apache.lucene.tests.util.QuickPatchThreadsFilter;
import org.apache.solr.SolrIgnoredThreadsFilter;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
import org.apache.solr.handler.extraction.fromtika.ToXMLContentHandler;
import org.junit.ClassRule;
import org.junit.Test;
@@ -195,4 +200,64 @@ public class TikaServerExtractionBackendTest extends
SolrTestCaseJ4 {
}
}
}
+
+ /**
+ * Races the last backend's close() (which stops the shared HttpClient)
against construction of a
+ * new backend, which must end up with a working client rather than the one
being stopped.
+ */
+ @Test
+ public void testConcurrentCloseAndAcquireSharedResources() throws Exception {
+ String baseUrl = tikaContainer.getBaseUrl();
+ int iterations = atLeast(25);
+ ExecutorService exec =
+ ExecutorUtil.newMDCAwareFixedThreadPool(2, new
SolrNamedThreadFactory("tikaRace"));
+ try {
+ for (int i = 0; i < iterations; i++) {
+ TikaServerExtractionBackend last = new
TikaServerExtractionBackend(baseUrl);
+ CyclicBarrier barrier = new CyclicBarrier(2);
+ Future<?> closer =
+ exec.submit(
+ () -> {
+ barrier.await();
+ last.close();
+ return null;
+ });
+ Future<TikaServerExtractionBackend> opener =
+ exec.submit(
+ () -> {
+ barrier.await();
+ return new TikaServerExtractionBackend(baseUrl);
+ });
+ closer.get();
+ try (TikaServerExtractionBackend fresh = opener.get()) {
+ assertExtracts(fresh, "iteration " + i);
+ }
+ }
+ } finally {
+ ExecutorUtil.shutdownAndAwaitTermination(exec);
+ }
+ }
+
+ @Test
+ public void testCloseIsIdempotent() throws Exception {
+ String baseUrl = tikaContainer.getBaseUrl();
+ TikaServerExtractionBackend first = new
TikaServerExtractionBackend(baseUrl);
+ try (TikaServerExtractionBackend second = new
TikaServerExtractionBackend(baseUrl)) {
+ first.close();
+ first.close();
+ assertExtracts(second, "after double close");
+ }
+ try (TikaServerExtractionBackend third = new
TikaServerExtractionBackend(baseUrl)) {
+ assertExtracts(third, "after all closed");
+ }
+ }
+
+ private void assertExtracts(TikaServerExtractionBackend backend, String
marker) throws Exception {
+ byte[] data = ("Hello " + marker).getBytes(StandardCharsets.UTF_8);
+ try (ByteArrayInputStream in = new ByteArrayInputStream(data)) {
+ ExtractionResult res = backend.extract(in, newRequest("test.txt",
"text/plain", "text"));
+ assertNotNull(res);
+ assertTrue(res.getContent().contains("Hello " + marker));
+ }
+ }
}
