This is an automated email from the ASF dual-hosted git repository.
ffang pushed a commit to branch 3.6.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/3.6.x-fixes by this push:
new 06c1cb20d0a CXF-9265: Closing or finalizing a JAX-RS client proxy
closes the conduit of its sub-resource proxies (#3659)
06c1cb20d0a is described below
commit 06c1cb20d0ad96da4f7f8afb3cd3a7b001013e1d
Author: Freeman(Yue) Fang <[email protected]>
AuthorDate: Sun Nov 1 14:26:51 2026 -0500
CXF-9265: Closing or finalizing a JAX-RS client proxy closes the conduit of
its sub-resource proxies (#3659)
A sub-resource proxy uses the same ClientConfiguration, and so the same
conduit, as the proxy that returned it. ClientProxyImpl handed the
configuration over with setConfiguration(), which starts a new configuration
reference, so each proxy closed the shared conduit as if it were the only
one
using it. Closing the parent proxy then failed the calls of the sub-resource
proxy, and when both proxies were dropped, their finalizers closed the
conduit
twice, which could shut down the HttpClient shared with other conduits.
Share the parent's configuration reference with the sub-resource proxy, as
WebClient.fromClient() does since CXF-8992, so that the conduit is closed
only
when the last proxy using it is closed.
Also make HttpClientHTTPConduit.close() release its HttpClient reference
only
once when the conduit is closed concurrently.
(cherry picked from commit 5f690d89bd267bc21c94921ae201c436e718d579)
(cherry picked from commit f24267459a16088ac019e50c7e5c72a7ad9c2327)
---
.../apache/cxf/jaxrs/client/ClientProxyImpl.java | 2 +-
.../jaxrs/client/JAXRSClientFactoryBeanTest.java | 29 +++++-
.../cxf/transport/http/HttpClientHTTPConduit.java | 18 +++-
.../http/HttpClientHTTPConduitCloseTest.java | 113 +++++++++++++++++++++
4 files changed, 155 insertions(+), 7 deletions(-)
diff --git
a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/ClientProxyImpl.java
b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/ClientProxyImpl.java
index 22dcdc4d494..1feda4d5187 100644
---
a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/ClientProxyImpl.java
+++
b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/ClientProxyImpl.java
@@ -320,7 +320,7 @@ public class ClientProxyImpl extends AbstractClient
implements
ClientState newState = getState().newState(uri, subHeaders,
getTemplateParametersMap(ori.getURITemplate(), pathParams));
ClientProxyImpl proxyImpl = createProxy(newState, proxyLoader,
subCri, false, inheritHeaders);
- proxyImpl.setConfiguration(getConfiguration());
+ proxyImpl.setConfigurationReference(getConfigurationReference());
return JAXRSClientFactory.createProxy(m.getReturnType(),
proxyLoader, proxyImpl);
}
headers.putAll(paramHeaders);
diff --git
a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java
b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java
index ec1424caabb..29e84f4a300 100644
---
a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java
+++
b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java
@@ -299,13 +299,17 @@ public class JAXRSClientFactoryBeanTest {
final Client client = (Client) superBookResource;
final WebClient wc = WebClient.fromClient(client);
+ // a sub-resource proxy
final Book book = superBookResource.getNewBook("id4", true);
assertNotNull(book);
-
+
assertThat(wc.getConfigurationReference(), is(not(nullValue())));
- assertThat(wc.getConfigurationReference().refCount(), equalTo(2L));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(3L));
client.close();
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(2L));
+
+ WebClient.client(book).close();
assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
wc.close();
@@ -313,6 +317,27 @@ public class JAXRSClientFactoryBeanTest {
}
+ @Test
+ public void testSubresourceProxySharesConfigurationReference() throws
Exception {
+ JAXRSClientFactoryBean bean = new JAXRSClientFactoryBean();
+ bean.setAddress("http://bar";);
+ bean.setResourceClass(BookStoreSubresourcesOnly.class);
+ BookStoreSubresourcesOnly store =
bean.create(BookStoreSubresourcesOnly.class, 1, 2, 3);
+ BookStoreSubresourcesOnly store2 = store.getItself();
+
+ final AbstractClient root =
(AbstractClient)((InvocationHandlerAware)store).getInvocationHandler();
+ final AbstractClient sub =
(AbstractClient)((InvocationHandlerAware)store2).getInvocationHandler();
+ assertSame(root.getConfigurationReference(),
sub.getConfigurationReference());
+ assertThat(sub.getConfigurationReference().refCount(), equalTo(2L));
+
+ // closing the root proxy must not shut down the configuration the
sub-resource proxy uses
+ WebClient.client(store).close();
+ assertThat(sub.getConfigurationReference().refCount(), equalTo(1L));
+
+ WebClient.client(store2).close();
+ assertThat(sub.getConfigurationReference(), is(nullValue()));
+ }
+
private final class TestFeature extends AbstractFeature {
private TestInterceptor testInterceptor;
diff --git
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpClientHTTPConduit.java
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpClientHTTPConduit.java
index 040db357cba..675613eb2cc 100644
---
a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpClientHTTPConduit.java
+++
b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpClientHTTPConduit.java
@@ -289,12 +289,22 @@ public class HttpClientHTTPConduit extends
URLConnectionHTTPConduit {
* Close the conduit
*/
public void close() {
- if (clientRef != null) {
- clientRef.release();
+ // release each reference only once, even if the conduit is closed
concurrently
+ final RefCount<HttpClient> ref;
+ initializationLock.lock();
+ try {
+ ref = clientRef;
clientRef = null;
+ } finally {
+ initializationLock.unlock();
+ }
+ if (ref != null) {
+ ref.release();
+ }
+ RefCount<HttpClient> deferred;
+ while ((deferred = deferredClientRefs.poll()) != null) {
+ deferred.release();
}
- deferredClientRefs.forEach(RefCount::release);
- deferredClientRefs.clear();
defaultAddress = null;
super.close();
}
diff --git
a/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HttpClientHTTPConduitCloseTest.java
b/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HttpClientHTTPConduitCloseTest.java
new file mode 100644
index 00000000000..5bc1aae9720
--- /dev/null
+++
b/rt/transports/http/src/test/java/org/apache/cxf/transport/http/HttpClientHTTPConduitCloseTest.java
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cxf.transport.http;
+
+import java.net.http.HttpClient;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+
+import org.apache.cxf.Bus;
+import org.apache.cxf.bus.extension.ExtensionManagerBus;
+import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageImpl;
+import org.apache.cxf.service.model.EndpointInfo;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
+
+public class HttpClientHTTPConduitCloseTest {
+ private static final int THREADS = 4;
+
+ private Bus bus;
+ private ExecutorService executor;
+
+ @Before
+ public void setUp() {
+ bus = new ExtensionManagerBus();
+ executor = Executors.newFixedThreadPool(THREADS);
+ }
+
+ @After
+ public void tearDown() {
+ executor.shutdownNow();
+ bus.shutdown(true);
+ }
+
+ /**
+ * Closing a conduit from several threads at the same time, as two
finalizers of clients sharing
+ * the conduit may do, must release its shared HttpClient only once, so
that the HttpClient
+ * stays open for the other conduits that use it.
+ */
+ @Test
+ public void testConcurrentCloseReleasesSharedClientOnce() throws Exception
{
+ for (int i = 0; i < 1000; i++) {
+ HttpClientHTTPConduit other = newConduit();
+ HttpClient client = prepare(other);
+
+ HttpClientHTTPConduit closed = newConduit();
+ assertSame(client, prepare(closed));
+
+ CyclicBarrier barrier = new CyclicBarrier(THREADS);
+ List<Future<?>> closes = new ArrayList<>();
+ for (int t = 0; t < THREADS; t++) {
+ closes.add(executor.submit(() -> {
+ barrier.await();
+ closed.close();
+ return null;
+ }));
+ }
+ for (Future<?> close : closes) {
+ close.get();
+ }
+
+ // the HttpClient is still shared, so it was not shut down
+ HttpClientHTTPConduit next = newConduit();
+ assertSame("Iteration " + i, client, prepare(next));
+
+ next.close();
+ other.close();
+ }
+ }
+
+ private HttpClientHTTPConduit newConduit() throws Exception {
+ EndpointInfo ei = new EndpointInfo();
+ ei.setAddress("http://localhost/test";);
+ HttpClientHTTPConduit conduit = new HttpClientHTTPConduit(bus, ei,
null);
+ conduit.finalizeConfig();
+ return conduit;
+ }
+
+ private static HttpClient prepare(HttpClientHTTPConduit conduit) throws
Exception {
+ Message message = new MessageImpl();
+ message.put(Message.HTTP_REQUEST_METHOD, "GET");
+ conduit.prepare(message);
+ HttpClient client = message.get(HttpClient.class);
+ assertNotNull(client);
+ return client;
+ }
+}
