You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by re...@apache.org on 2025/03/02 13:09:27 UTC
(cxf) branch 3.5.x-fixes updated (cd6672bc6a -> 8f670cd8c9)
This is an automated email from the ASF dual-hosted git repository.
reta pushed a change to branch 3.5.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
from cd6672bc6a Recording .gitmergeinfo Changes
new 3199140d6c CXF-8992: WebClient.fromClient() broken due to garbage collection (#2281)
new 8f670cd8c9 Recording .gitmergeinfo Changes
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.gitmergeinfo | 4 +
.../apache/cxf/jaxrs/client/AbstractClient.java | 85 +++++++++--
.../org/apache/cxf/jaxrs/client/WebClient.java | 19 ++-
.../jaxrs/client/JAXRSClientFactoryBeanTest.java | 43 ++++++
.../org/apache/cxf/jaxrs/client/WebClientTest.java | 166 ++++++++++++++++++++-
5 files changed, 304 insertions(+), 13 deletions(-)
(cxf) 02/02: Recording .gitmergeinfo Changes
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
reta pushed a commit to branch 3.5.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 8f670cd8c9c25ed91a0d119dc515b51461f95c7a
Author: Andriy Redko <dr...@gmail.com>
AuthorDate: Sun Mar 2 07:58:50 2025 -0500
Recording .gitmergeinfo Changes
---
.gitmergeinfo | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/.gitmergeinfo b/.gitmergeinfo
index 84c020938e..87a2988e96 100644
--- a/.gitmergeinfo
+++ b/.gitmergeinfo
@@ -146,6 +146,7 @@ B 4321cb82c67aff9265c26dc42ab6ec84493729bd
B 43b5beab03ae26a964a437b8ad9423b681cbfd6f
B 44919916b3747beb059cadd95a34e0bf15202a0d
B 44b60ff6a49811ebfe2cd94b6aff64e785bd3767
+B 44c8219cad592eb2f3cf3583829e7ada628d45da
B 450be91e2fa03528baae61006a188d3624c19565
B 45188882b192a65b2572274ebc7faad677ecb554
B 453287d2990a2b417b498ddf178f05d6b5e475da
@@ -698,6 +699,7 @@ M 31872c9456ed871f05b31abbb68318645b3816f7
M 31e00d4ec03b22d358af62fe0cb2b8785c856400
M 31f6f59502e16bc6c16817d246a9e08349f48da7
M 32686bc0e57f3b67dd08d4681c588e3e504cef97
+M 3323d4607783743d0ef900e2b546f92d07209b3e
M 338818fa1c5f3aee7d533199797b61a694dcbb46
M 34fb77e6ee1d061ebe5fc24e5f01d578ae31ae4f
M 35635cb20c474945adfe02f6b3e6d8d7dbf52b19
@@ -746,6 +748,7 @@ M 410728df7c306a759afb74e51c609ccac86920b5
M 410a7aedabfc85a99f605138ad717b6bd3a51c07
M 4149f832ca3a0868928f9978f75c1eae4d8e187d
M 4332f0fd99bb4b043cec55fdae4a75c5bfdb1cab
+M 435e8453d7992ed8b708721984d82ccea50e8333
M 43b4eee96cbffdf17218c235aa7f5956a1df053f
M 43f323705ec3bc9d7615c2333ede5b4681288150
M 4410892ad37e906863e7a996f543c5b503dcc666
@@ -898,6 +901,7 @@ M 737a3fdc61bbc0d87b681da721f6b9bb233ef217
M 73db1a3eaf035913f783cbdafaa4ef4235118107
M 73f1c51e320076a85f4bd8a728298ca73520ad67
M 74e0559ac8a2333e25a75f0cbf11bff2567b84d5
+M 74f1b87836f1f5872740149493d3d0c818223650
M 7501891ee05153ba43eab3cdadccf789dd15de1e
M 750494493d5d84445ae3314af7e3e0f6108f3241
M 7542c0314bcdd82251b8677d9c09b6f136ce2310
(cxf) 01/02: CXF-8992: WebClient.fromClient() broken due to garbage collection (#2281)
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
reta pushed a commit to branch 3.5.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 3199140d6cd27e3c5b204f4ab9918203938504b4
Author: Andriy Redko <dr...@gmail.com>
AuthorDate: Sat Mar 1 20:58:13 2025 -0500
CXF-8992: WebClient.fromClient() broken due to garbage collection (#2281)
(cherry picked from commit 9756ab897d93ffd28cbd86487abd3ae0df19ec47)
# Conflicts:
# rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java
(cherry picked from commit fd0b8e9d1c6ee68e83870c393e4d0a3af8d0f2e8)
# Conflicts:
# rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
# rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java
(cherry picked from commit e4607d659bd8e8ed4da07125a53423caf6d3fc92)
---
.../apache/cxf/jaxrs/client/AbstractClient.java | 85 +++++++++--
.../org/apache/cxf/jaxrs/client/WebClient.java | 19 ++-
.../jaxrs/client/JAXRSClientFactoryBeanTest.java | 43 ++++++
.../org/apache/cxf/jaxrs/client/WebClientTest.java | 166 ++++++++++++++++++++-
4 files changed, 300 insertions(+), 13 deletions(-)
diff --git a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
index 8018f4b5be..a71ca21b93 100644
--- a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
+++ b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
@@ -41,6 +41,7 @@ import java.util.Set;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.logging.Logger;
import javax.ws.rs.PathParam;
@@ -130,6 +131,34 @@ public abstract class AbstractClient implements Client {
protected ClientConfiguration cfg = new ClientConfiguration();
private ClientState state;
private final AtomicBoolean closed = new AtomicBoolean();
+ private volatile ConfigurationReference configurationReference = new ConfigurationReference(cfg);
+
+ protected static final class ConfigurationReference {
+ private final AtomicLong refCount = new AtomicLong(1);
+ private final ClientConfiguration cfg;
+
+ public ConfigurationReference(ClientConfiguration cfg) {
+ this.cfg = cfg;
+ }
+
+ public ConfigurationReference acquire() {
+ refCount.incrementAndGet();
+ return this;
+ }
+
+ public long refCount() {
+ return refCount.get();
+ }
+
+ public long release() {
+ return refCount.decrementAndGet();
+ }
+
+ public ClientConfiguration getConfiguration() {
+ return cfg;
+ }
+ }
+
protected AbstractClient(ClientState initialState) {
this.state = initialState;
}
@@ -342,7 +371,11 @@ public abstract class AbstractClient implements Client {
if (cfg.getBus() == null) {
return;
}
- cfg.getEndpoint().getCleanupHooks().
+
+ final ConfigurationReference reference = configurationReference;
+ final boolean shutdownConfiguration = reference == null || reference.release() == 0L;
+ if (shutdownConfiguration) {
+ cfg.getEndpoint().getCleanupHooks().
forEach(c -> {
try {
c.close();
@@ -350,26 +383,35 @@ public abstract class AbstractClient implements Client {
//ignore
}
});
- ClientLifeCycleManager mgr = cfg.getBus().getExtension(ClientLifeCycleManager.class);
+ }
+
+ final ClientLifeCycleManager mgr = cfg.getBus().getExtension(ClientLifeCycleManager.class);
if (null != mgr) {
mgr.clientDestroyed(new FrontendClientAdapter(getConfiguration()));
}
- if (cfg.getConduitSelector() instanceof Closeable) {
- try {
- ((Closeable)cfg.getConduitSelector()).close();
- } catch (IOException e) {
- //ignore, we're destroying anyway
+ if (shutdownConfiguration) {
+ if (cfg.getConduitSelector() instanceof Closeable) {
+ try {
+ ((Closeable)cfg.getConduitSelector()).close();
+ } catch (IOException e) {
+ //ignore, we're destroying anyway
+ }
+ } else {
+ cfg.getConduit().close();
}
- } else {
- cfg.getConduit().close();
}
+
+ // reset state
state.reset();
- if (cfg.isShutdownBusOnClose()) {
+
+ if (shutdownConfiguration && cfg.isShutdownBusOnClose()) {
cfg.getBus().shutdown(false);
}
+
state = null;
cfg = null;
+ configurationReference = null;
}
}
@@ -901,6 +943,7 @@ public abstract class AbstractClient implements Client {
protected void setConfiguration(ClientConfiguration config) {
cfg = config;
+ configurationReference = new ConfigurationReference(config);
}
// Note that some conduit selectors may update Message.ENDPOINT_ADDRESS
@@ -1321,4 +1364,26 @@ public abstract class AbstractClient implements Client {
protected void handleAsyncFault(Message message) {
}
}
+
+ /**
+ * References the configuration (by another client) so it won't shut down till all the
+ * client instances are closed.
+ * @param reference configuration reference
+ */
+ protected void setConfigurationReference(ConfigurationReference reference) {
+ if (reference == null) {
+ throw new IllegalArgumentException("The configuration reference is not set "
+ + "(the client was already closed)");
+ }
+ this.configurationReference = reference.acquire();
+ this.cfg = configurationReference.getConfiguration();
+ }
+
+ /**
+ * Returns configuration reference
+ * @return configuration reference
+ */
+ protected ConfigurationReference getConfigurationReference() {
+ return this.configurationReference;
+ }
}
diff --git a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/WebClient.java b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/WebClient.java
index 9f5de1bf05..472ab1f170 100644
--- a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/WebClient.java
+++ b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/WebClient.java
@@ -82,6 +82,9 @@ import org.apache.cxf.message.Message;
*
*/
public class WebClient extends AbstractClient {
+ // Use client configuration reference instead of sharing the instance directly
+ public static final String USE_CONFIGURATION_REFERENCE_WHEN_COPY = "use.configuration.reference.when.copy";
+
private static final String REQUEST_CLASS = "request.class";
private static final String REQUEST_TYPE = "request.type";
private static final String REQUEST_ANNS = "request.annotations";
@@ -1233,7 +1236,21 @@ public class WebClient extends AbstractClient {
static void copyProperties(Client toClient, Client fromClient) {
AbstractClient newClient = toAbstractClient(toClient);
AbstractClient oldClient = toAbstractClient(fromClient);
- newClient.setConfiguration(oldClient.getConfiguration());
+ final ClientConfiguration oldCfg = oldClient.getConfiguration();
+
+ boolean useConfigurationReference = true;
+ if (oldCfg != null && oldCfg.getBus() != null) {
+ Object useConfigurationReferenceProp = oldCfg.getBus().getProperty(USE_CONFIGURATION_REFERENCE_WHEN_COPY);
+ if (useConfigurationReferenceProp != null) {
+ useConfigurationReference = PropertyUtils.isTrue(useConfigurationReferenceProp);
+ }
+ }
+
+ if (useConfigurationReference) {
+ newClient.setConfigurationReference(oldClient.getConfigurationReference());
+ } else {
+ newClient.setConfiguration(oldClient.getConfiguration());
+ }
}
private static AbstractClient toAbstractClient(Object client) {
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 b13061fd88..7f396c89b0 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
@@ -55,6 +55,11 @@ import org.apache.cxf.transport.http.HTTPConduit;
import org.junit.Test;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertSame;
@@ -269,6 +274,44 @@ public class JAXRSClientFactoryBeanTest {
assertNotNull(store.getBook(""));
}
+ @Test
+ public void testCreateClientFrom() throws Exception {
+ JAXRSClientFactoryBean bean = new JAXRSClientFactoryBean();
+ bean.setAddress("http://bar");
+ bean.setResourceClass(BookStore.class);
+
+ final Client client = bean.create();
+ final WebClient wc = WebClient.fromClient(client);
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(2L));
+
+ client.close();
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+
+ wc.close();
+ assertThat(wc.getConfigurationReference(), is(nullValue()));
+ }
+
+ @Test
+ public void testCreateClientFromAndInvoke() throws Exception {
+ final SuperBookStore superBookResource = JAXRSClientFactory
+ .create("http://localhost:9000", SuperBookStore.class);
+ final Client client = (Client) superBookResource;
+ final WebClient wc = WebClient.fromClient(client);
+
+ final Book book = superBookResource.getNewBook("id4", true);
+ assertNotNull(book);
+
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(2L));
+
+ client.close();
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+
+ wc.close();
+ assertThat(wc.getConfigurationReference(), is(nullValue()));
+
+ }
private final class TestFeature extends AbstractFeature {
private TestInterceptor testInterceptor;
diff --git a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java
index 9421e88f2e..303f6f9932 100644
--- a/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java
+++ b/rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java
@@ -22,26 +22,40 @@ package org.apache.cxf.jaxrs.client;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.net.URI;
+import java.util.ArrayList;
import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import javax.ws.rs.core.Cookie;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.ext.ParamConverter;
import javax.ws.rs.ext.ParamConverterProvider;
+import org.apache.cxf.Bus;
+import org.apache.cxf.Bus.BusState;
import org.apache.cxf.jaxrs.resources.BookInterface;
import org.apache.cxf.jaxrs.resources.BookStore;
import org.junit.Test;
+import static org.hamcrest.CoreMatchers.anyOf;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
-
public class WebClientTest {
@Test
@@ -365,6 +379,154 @@ public class WebClientTest {
containsInAnyOrder("a=1", "b=2"));
}
+ @Test
+ public void testWebClientClose() {
+ final WebClient wc = WebClient.create("http://foo").language("en_CA");
+ wc.getConfiguration().setShutdownBusOnClose(true);
+
+ final Bus bus = wc.getConfiguration().getBus();
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+ wc.close();
+
+ assertThat(wc.getConfigurationReference(), is(nullValue()));
+ assertThat(wc.getConfiguration(), is(nullValue()));
+
+ assertThat(bus.getState(), equalTo(BusState.SHUTDOWN));
+ }
+
+ @Test
+ public void testWebClientFromConcurrently() throws InterruptedException, ExecutionException, TimeoutException {
+ final WebClient wc = WebClient.create("http://foo").language("en_CA");
+ wc.getConfiguration().setShutdownBusOnClose(true);
+
+ final List<Future<WebClient>> futures = new ArrayList<>(100);
+ final ExecutorService executor = Executors.newFixedThreadPool(8);
+ for (int i = 0; i < 100; ++i) {
+ futures.add(executor.submit(() -> WebClient.fromClient(wc)));
+ }
+
+ final Bus bus = wc.getConfiguration().getBus();
+ final List<WebClient> clients = new ArrayList<>(100);
+ try {
+ for (Future<WebClient> future: futures) {
+ final WebClient client = future.get(5, TimeUnit.SECONDS);
+ assertThat(client, is(not(nullValue())));
+ clients.add(client);
+ }
+
+ assertThat(bus.getState(), anyOf(equalTo(BusState.RUNNING),
+ equalTo(BusState.INITIALIZING), equalTo(BusState.INITIAL)));
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(101L));
+
+ for (WebClient client: clients) {
+ executor.submit(client::close);
+ }
+ } finally {
+ executor.shutdown();
+ assertThat(executor.awaitTermination(5, TimeUnit.SECONDS), equalTo(true));
+ }
+
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+ wc.close();
+
+ assertThat(wc.getConfigurationReference(), is(nullValue()));
+ assertThat(wc.getConfiguration(), is(nullValue()));
+
+ assertThat(bus.getState(), equalTo(BusState.SHUTDOWN));
+ }
+
+ @Test
+ public void testWebClientFrom() {
+ final WebClient wc = WebClient.create("http://foo").language("en_CA");
+ wc.getConfiguration().setShutdownBusOnClose(true);
+
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+
+ final WebClient wc1 = WebClient.fromClient(wc);
+ assertThat(wc.getConfigurationReference(), equalTo(wc1.getConfigurationReference()));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(2L));
+ wc.close();
+
+ final ClientConfiguration configuration = wc1.getConfiguration();
+ assertThat(configuration, is(not(nullValue())));
+ assertThat(configuration.getBus().getState(), anyOf(equalTo(BusState.RUNNING),
+ equalTo(BusState.INITIALIZING), equalTo(BusState.INITIAL)));
+
+ assertThat(wc.getConfigurationReference(), is(nullValue()));
+ assertThat(wc1.getConfigurationReference().refCount(), equalTo(1L));
+ wc1.close();
+
+ assertThat(wc1.getConfigurationReference(), is(nullValue()));
+ assertThat(configuration.getBus().getState(), equalTo(BusState.SHUTDOWN));
+ }
+
+ @Test
+ public void testWebClientFromChained() {
+ final WebClient wc = WebClient.create("http://foo").language("en_CA");
+ wc.getConfiguration().setShutdownBusOnClose(true);
+
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+
+ final WebClient wc1 = WebClient.fromClient(wc);
+ assertThat(wc.getConfigurationReference(), equalTo(wc1.getConfigurationReference()));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(2L));
+
+ final WebClient wc2 = WebClient.fromClient(wc);
+ assertThat(wc.getConfigurationReference(), equalTo(wc2.getConfigurationReference()));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(3L));
+ wc.close();
+
+ final ClientConfiguration configuration1 = wc1.getConfiguration();
+ assertThat(configuration1, is(not(nullValue())));
+ assertThat(configuration1.getBus().getState(), anyOf(equalTo(BusState.RUNNING),
+ equalTo(BusState.INITIALIZING), equalTo(BusState.INITIAL)));
+ assertThat(wc1.getConfigurationReference().refCount(), equalTo(2L));
+
+ final ClientConfiguration configuration2 = wc2.getConfiguration();
+ assertThat(configuration2, is(not(nullValue())));
+ assertThat(configuration2.getBus(), is(configuration1.getBus()));
+ wc1.close();
+
+ assertThat(wc.getConfigurationReference(), is(nullValue()));
+ assertThat(wc2.getConfigurationReference().refCount(), equalTo(1L));
+ wc2.close();
+
+ assertThat(wc2.getConfigurationReference(), is(nullValue()));
+ assertThat(configuration2.getBus().getState(), equalTo(BusState.SHUTDOWN));
+ }
+
+ @Test
+ public void testWebClientFromShare() {
+ final WebClient wc = WebClient.create("http://foo").language("en_CA");
+ wc.getConfiguration().setShutdownBusOnClose(true);
+ wc.getConfiguration().getBus().setProperty(WebClient.USE_CONFIGURATION_REFERENCE_WHEN_COPY, false);
+
+ assertThat(wc.getConfigurationReference(), is(not(nullValue())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+
+ final WebClient wc1 = WebClient.fromClient(wc);
+ assertThat(wc.getConfigurationReference(), not(equalTo(wc1.getConfigurationReference())));
+ assertThat(wc.getConfigurationReference().refCount(), equalTo(1L));
+ assertThat(wc1.getConfigurationReference().refCount(), equalTo(1L));
+ wc.close();
+
+ final ClientConfiguration configuration = wc1.getConfiguration();
+ assertThat(configuration, is(not(nullValue())));
+ assertThat(configuration.getBus().getState(), equalTo(BusState.SHUTDOWN));
+
+ assertThat(wc.getConfigurationReference(), is(nullValue()));
+ assertThat(wc1.getConfigurationReference().refCount(), equalTo(1L));
+ wc1.close();
+
+ assertThat(wc1.getConfigurationReference(), is(nullValue()));
+ assertThat(configuration.getBus().getState(), equalTo(BusState.SHUTDOWN));
+ }
+
private static final class ParamConverterProviderImpl implements ParamConverterProvider {
@SuppressWarnings("unchecked")