From 683c92c19614833dd65914139c6ea44f5e419ec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Mon, 15 Sep 2025 11:15:20 +0200 Subject: [PATCH 1/2] GH-4673 Add failing test for Cleaner-based auto shutdown in SPARQLRepository --- .../sparql/SPARQLRepositoryCleanerTest.java | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java diff --git a/core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java b/core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java new file mode 100644 index 00000000000..45d103532ff --- /dev/null +++ b/core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java @@ -0,0 +1,72 @@ +/******************************************************************************* + * Copyright (c) 2025 Eclipse RDF4J contributors. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Distribution License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + *******************************************************************************/ +package org.eclipse.rdf4j.repository.sparql; + +import static org.assertj.core.api.Assertions.fail; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.eclipse.rdf4j.repository.RepositoryConnection; +import org.eclipse.rdf4j.repository.RepositoryException; +import org.junit.jupiter.api.Test; + +/** + * Verifies that a SPARQLRepository performs internal shutdown when it becomes unreachable. + * + *

+ * This test intentionally does not call {@code repository.shutDown()}. It expects the repository to arrange for its + * internal {@code shutDownInternal()} to run when the object is no longer reachable (e.g., by using Java 9 Cleaner). + *

+ */ +public class SPARQLRepositoryCleanerTest { + + @Test + void autoShutdownOnUnreachable() throws Exception { + CountDownLatch shutdownInvoked = new CountDownLatch(1); + + runRepo(shutdownInvoked); + + // Encourage GC and wait briefly for cleaner to run + boolean observed = false; + for (int i = 0; i < 20 && !observed; i++) { + System.gc(); + System.runFinalization(); + observed = shutdownInvoked.await(250, TimeUnit.MILLISECONDS); + } + + if (!observed) { + fail("Expected shutDownInternal() to be invoked when SPARQLRepository became unreachable"); + } + } + + private static void runRepo(CountDownLatch shutdownInvoked) { + // Create a repository instance that signals when shutDownInternal() is invoked + SPARQLRepository repo = new SPARQLRepository("http://example.org/sparql") { + @Override + protected void shutDownInternal() throws RepositoryException { + try { + super.shutDownInternal(); + } finally { + shutdownInvoked.countDown(); + } + } + }; + + // Exercise minimal usage without hitting the network + try (RepositoryConnection conn = repo.getConnection()) { + // no-op + } + + // Drop strong reference and ask GC to collect + repo = null; + } +} From 7d991dc422a51555c8e03dd4f7997dd493eb9960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Avard=20Ottestad?= Date: Mon, 15 Sep 2025 11:23:33 +0200 Subject: [PATCH 2/2] GH-4673 Use ConcurrentCleaner to auto-shutdown SPARQLRepository's dependent session manager on GC; add test --- .../repository/sparql/SPARQLRepository.java | 31 +++++++++- .../sparql/SPARQLRepositoryCleanerTest.java | 58 ++++++++----------- 2 files changed, 55 insertions(+), 34 deletions(-) diff --git a/core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepository.java b/core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepository.java index 9a68f3381c1..5e7974a2bba 100644 --- a/core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepository.java +++ b/core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepository.java @@ -11,10 +11,12 @@ package org.eclipse.rdf4j.repository.sparql; import java.io.File; +import java.lang.ref.Cleaner; import java.util.Collections; import java.util.Map; import org.apache.http.client.HttpClient; +import org.eclipse.rdf4j.common.concurrent.locks.diagnostics.ConcurrentCleaner; import org.eclipse.rdf4j.http.client.HttpClientDependent; import org.eclipse.rdf4j.http.client.HttpClientSessionManager; import org.eclipse.rdf4j.http.client.SPARQLProtocolSession; @@ -33,6 +35,8 @@ */ public class SPARQLRepository extends AbstractRepository implements HttpClientDependent, SessionManagerDependent { + private static final ConcurrentCleaner CLEANER = new ConcurrentCleaner(); + /** * Flag indicating if quad mode is enabled in newly created {@link SPARQLConnection}s. * @@ -49,6 +53,11 @@ public class SPARQLRepository extends AbstractRepository implements HttpClientDe */ private volatile SharedHttpClientSessionManager dependentClient; + /** + * Cleanable registration to auto-invoke cleanup when this repository becomes unreachable. + */ + private volatile Cleaner.Cleanable cleanable; + private String username; private String password; @@ -93,7 +102,17 @@ public HttpClientSessionManager getHttpClientSessionManager() { synchronized (this) { result = client; if (result == null) { - result = client = dependentClient = new SharedHttpClientSessionManager(); + SharedHttpClientSessionManager created = new SharedHttpClientSessionManager(); + result = client = dependentClient = created; + // Register a cleaner that shuts down the dependent client if this repository is GC'ed without + // explicit shutdown + cleanable = CLEANER.register(this, () -> { + try { + created.shutDown(); + } catch (Throwable t) { + // ignore + } + }); } } } @@ -110,6 +129,11 @@ public void setHttpClientSessionManager(HttpClientSessionManager client) { if (toCloseDependentClient != null) { toCloseDependentClient.shutDown(); } + Cleaner.Cleanable toClean = cleanable; + cleanable = null; + if (toClean != null) { + toClean.clean(); + } } } @@ -213,6 +237,11 @@ protected void shutDownInternal() throws RepositoryException { toCloseDependentClient.shutDown(); } } finally { + Cleaner.Cleanable toClean = cleanable; + cleanable = null; + if (toClean != null) { + toClean.clean(); + } // remove reference but do not shut down, client may be shared by // other repos. client = null; diff --git a/core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java b/core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java index 45d103532ff..bb8060288e8 100644 --- a/core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java +++ b/core/repository/sparql/src/test/java/org/eclipse/rdf4j/repository/sparql/SPARQLRepositoryCleanerTest.java @@ -10,13 +10,14 @@ *******************************************************************************/ package org.eclipse.rdf4j.repository.sparql; -import static org.assertj.core.api.Assertions.fail; +import static org.assertj.core.api.Assertions.assertThat; -import java.util.concurrent.CountDownLatch; +import java.lang.reflect.Field; +import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; +import org.eclipse.rdf4j.http.client.SharedHttpClientSessionManager; import org.eclipse.rdf4j.repository.RepositoryConnection; -import org.eclipse.rdf4j.repository.RepositoryException; import org.junit.jupiter.api.Test; /** @@ -31,42 +32,33 @@ public class SPARQLRepositoryCleanerTest { @Test void autoShutdownOnUnreachable() throws Exception { - CountDownLatch shutdownInvoked = new CountDownLatch(1); + SPARQLRepository repo = new SPARQLRepository("http://example.org/sparql"); - runRepo(shutdownInvoked); - - // Encourage GC and wait briefly for cleaner to run - boolean observed = false; - for (int i = 0; i < 20 && !observed; i++) { - System.gc(); - System.runFinalization(); - observed = shutdownInvoked.await(250, TimeUnit.MILLISECONDS); + // Ensure dependent client is created + try (RepositoryConnection conn = repo.getConnection()) { + // no-op } - if (!observed) { - fail("Expected shutDownInternal() to be invoked when SPARQLRepository became unreachable"); - } - } + SharedHttpClientSessionManager mgr = (SharedHttpClientSessionManager) repo.getHttpClientSessionManager(); - private static void runRepo(CountDownLatch shutdownInvoked) { - // Create a repository instance that signals when shutDownInternal() is invoked - SPARQLRepository repo = new SPARQLRepository("http://example.org/sparql") { - @Override - protected void shutDownInternal() throws RepositoryException { - try { - super.shutDownInternal(); - } finally { - shutdownInvoked.countDown(); - } - } - }; + // Access internal executor to verify shutdown state + Field f = SharedHttpClientSessionManager.class.getDeclaredField("executor"); + f.setAccessible(true); + ExecutorService exec = (ExecutorService) f.get(mgr); - // Exercise minimal usage without hitting the network - try (RepositoryConnection conn = repo.getConnection()) { - // no-op + // Drop strong reference and encourage GC to trigger Cleaner + repo = null; + + boolean cleaned = false; + for (int i = 0; i < 40 && !cleaned; i++) { + System.gc(); + System.runFinalization(); + TimeUnit.MILLISECONDS.sleep(100); + cleaned = exec.isShutdown() || exec.isTerminated(); } - // Drop strong reference and ask GC to collect - repo = null; + assertThat(cleaned) + .as("dependent session manager executor should be shut down by Cleaner") + .isTrue(); } }