CXF-8992: WebClient.fromClient() broken due to garbage collection

rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java

@@ -41,10 +41,12 @@
 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.xml.stream.XMLStreamWriter;
 
+import jakarta.annotation.Nullable;
 import jakarta.ws.rs.PathParam;
 import jakarta.ws.rs.ProcessingException;
 import jakarta.ws.rs.WebApplicationException;
@@ -130,6 +132,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;
     }
@@ -349,34 +379,46 @@ public void close() {
             if (cfg.getBus() == null) {
                 return;
             }
-            cfg.getEndpoint().getCleanupHooks().
+
+            final boolean shutdownConfiguration = configurationReference.release() == 0L;
+            if (shutdownConfiguration) {
+                cfg.getEndpoint().getCleanupHooks().
                     forEach(c -> {
                         try {
                             c.close();
                         } catch (IOException e) {
                             //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;
         }
     }
 
@@ -908,6 +950,7 @@ public ClientConfiguration getConfiguration() {
 
     protected void setConfiguration(ClientConfiguration config) {
         cfg = config;
+        configurationReference = new ConfigurationReference(config);
     }
 
     // Note that some conduit selectors may update Message.ENDPOINT_ADDRESS
@@ -1328,4 +1371,26 @@ protected void closeAsyncResponseIfPossible(Response r, Message outMessage, Jaxr
         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 @Nullable ConfigurationReference getConfigurationReference() {
+        return this.configurationReference;
+    }
 }

rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/WebClient.java

@@ -1232,7 +1232,7 @@ protected void doWriteBody(Message outMessage,
     static void copyProperties(Client toClient, Client fromClient) {
         AbstractClient newClient = toAbstractClient(toClient);
         AbstractClient oldClient = toAbstractClient(fromClient);
-        newClient.setConfiguration(oldClient.getConfiguration());
+        newClient.setConfigurationReference(oldClient.getConfigurationReference());
     }
 
     private static AbstractClient toAbstractClient(Object client) {

rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/JAXRSClientFactoryBeanTest.java

@@ -54,6 +54,11 @@
 
 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;
@@ -268,6 +273,44 @@ public void testInvokePathEmptyAllowed() throws Exception {
         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;

rt/rs/client/src/test/java/org/apache/cxf/jaxrs/client/WebClientTest.java

@@ -28,19 +28,24 @@
 import jakarta.ws.rs.core.HttpHeaders;
 import jakarta.ws.rs.ext.ParamConverter;
 import jakarta.ws.rs.ext.ParamConverterProvider;
+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
@@ -373,6 +378,32 @@ public void testCookieBuilder() {
             containsInAnyOrder("$Version=1;a=1", "$Version=1;b=2"));
     }
 
+    @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));
+    }
+
     private static final class ParamConverterProviderImpl implements ParamConverterProvider {
 
         @SuppressWarnings("unchecked")