diff --git a/src/main/java/org/metricshub/winrm/service/WinRMService.java b/src/main/java/org/metricshub/winrm/service/WinRMService.java index 1df15c6..6819c3c 100644 --- a/src/main/java/org/metricshub/winrm/service/WinRMService.java +++ b/src/main/java/org/metricshub/winrm/service/WinRMService.java @@ -56,6 +56,7 @@ import org.apache.cxf.Bus.BusState; import org.apache.cxf.BusFactory; import org.apache.cxf.endpoint.Client; +import org.apache.cxf.transport.http.HTTPConduitFactory; import org.apache.cxf.transport.http.asyncclient.AsyncHTTPConduit; import org.apache.cxf.transport.http.asyncclient.AsyncHTTPConduitFactory; import org.apache.cxf.transport.http.asyncclient.AsyncHTTPConduitFactory.UseAsyncPolicy; @@ -352,10 +353,12 @@ public void close() { } if (cmdClient != null) { + shutdownConduitFactory(cmdClient); cmdClient.destroy(); } if (wqlClient != null) { + shutdownConduitFactory(wqlClient); wqlClient.destroy(); } @@ -365,6 +368,20 @@ public void close() { } } + /** + * Retrieves the {@link AsyncHTTPConduitFactory} registered on the given client's endpoint and calls + * {@link AsyncHTTPConduitFactory#shutdown()} on it to stop any background threads (e.g. the idle-connection + * reaper thread). This must be done before destroying the client to prevent thread leaks. + * + * @param client the CXF {@link Client} whose conduit factory should be shut down + */ + private void shutdownConduitFactory(final Client client) { + final Object factory = client.getEndpoint().getEndpointInfo().getProperty(HTTPConduitFactory.class.getName()); + if (factory instanceof AsyncHTTPConduitFactory) { + ((AsyncHTTPConduitFactory) factory).shutdown(); + } + } + @Override public WindowsRemoteCommandResult executeCommand( final String command, diff --git a/src/main/java/org/metricshub/winrm/service/client/WinRMInvocationHandler.java b/src/main/java/org/metricshub/winrm/service/client/WinRMInvocationHandler.java index 52d727d..f90de91 100644 --- a/src/main/java/org/metricshub/winrm/service/client/WinRMInvocationHandler.java +++ b/src/main/java/org/metricshub/winrm/service/client/WinRMInvocationHandler.java @@ -50,6 +50,7 @@ import org.apache.cxf.frontend.ClientProxy; import org.apache.cxf.jaxws.JaxWsProxyFactoryBean; import org.apache.cxf.message.Message; +import org.apache.cxf.service.model.EndpointInfo; import org.apache.cxf.service.model.ServiceInfo; import org.apache.cxf.transport.http.HTTPConduitFactory; import org.apache.cxf.transport.http.asyncclient.AsyncHTTPConduit; @@ -403,10 +404,18 @@ static Client getWebServiceClient( client.getOutInterceptors().add(new SignAndEncryptOutInterceptor()); // this is different to endpoint properties - client - .getEndpoint() - .getEndpointInfo() - .setProperty(HTTPConduitFactory.class.getName(), new AsyncHttpEncryptionAwareConduitFactory()); + // Register the conduit factory only once: on authentication retries this method is re-invoked on the + // same client, whose cached conduit keeps using the factory it was created with. Replacing the property + // would orphan factory instances, and shutting down the in-use factory would silently downgrade the + // conduit to the synchronous transport (AsyncHTTPConduit.setupConnection checks factory.isShutdown()). + // Reusing the factory also guarantees WinRMService.close() shuts down the instance that owns the + // background threads. + final EndpointInfo endpointInfo = client.getEndpoint().getEndpointInfo(); + if ( + !(endpointInfo.getProperty(HTTPConduitFactory.class.getName()) instanceof AsyncHttpEncryptionAwareConduitFactory) + ) { + endpointInfo.setProperty(HTTPConduitFactory.class.getName(), new AsyncHttpEncryptionAwareConduitFactory()); + } final ServiceInfo serviceInfo = client.getEndpoint().getEndpointInfo().getService(); serviceInfo.setProperty("soap.force.doclit.bare", true); diff --git a/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java b/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java index 27488c7..543056a 100644 --- a/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java +++ b/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java @@ -31,6 +31,11 @@ import java.nio.file.Paths; import java.util.List; import org.apache.cxf.Bus; +import org.apache.cxf.endpoint.Client; +import org.apache.cxf.endpoint.Endpoint; +import org.apache.cxf.service.model.EndpointInfo; +import org.apache.cxf.transport.http.HTTPConduitFactory; +import org.apache.cxf.transport.http.asyncclient.AsyncHTTPConduitFactory; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -268,4 +273,76 @@ void testExecuteWql() throws Exception { assertEquals(emptyList(), winRMService.executeWql(wqlQuery, timeout)); } } + + @Test + void testCloseShutdownsConduitFactories() throws Exception { + // Use a unique endpoint to avoid interference with other test stubs + final WinRMEndpoint endpointForFactoryTest = new WinRMEndpoint( + null, + "factory-test-host", + null, + "user", + "pwd".toCharArray(), + null + ); + + // Set up a mock factory that will be stored in the client's endpoint info + final AsyncHTTPConduitFactory mockFactory = mock(AsyncHTTPConduitFactory.class); + + // Set up mock endpoint info containing the factory + final EndpointInfo mockEndpointInfo = mock(EndpointInfo.class); + doReturn(mockFactory).when(mockEndpointInfo).getProperty(HTTPConduitFactory.class.getName()); + + // Set up mock endpoint + final Endpoint mockEndpoint = mock(Endpoint.class); + doReturn(mockEndpointInfo).when(mockEndpoint).getEndpointInfo(); + + // Set up mock client + final Client mockClient = mock(Client.class); + doReturn(mockEndpoint).when(mockClient).getEndpoint(); + + // Set up mock invocation handlers that expose the configured client + final WinRMInvocationHandler cmdHandler = mock(WinRMInvocationHandler.class); + doReturn(mockClient).when(cmdHandler).getClient(); + + final WinRMInvocationHandler wqlHandler = mock(WinRMInvocationHandler.class); + doReturn(mockClient).when(wqlHandler).getClient(); + + // Override the default stubs for this specific endpoint (registered later, so they take precedence) + MOCKED_WIN_RM_SERVICE + .when(() -> + WinRMService.createWinRMInvocationHandlerInstance( + eq(endpointForFactoryTest), + any(Bus.class), + anyLong(), + isNull(), + isNull(), + anyList() + ) + ) + .thenReturn(cmdHandler); + + MOCKED_WIN_RM_SERVICE + .when(() -> + WinRMService.createWinRMInvocationHandlerInstance( + eq(endpointForFactoryTest), + any(Bus.class), + anyLong(), + anyString(), + isNull(), + anyList() + ) + ) + .thenReturn(wqlHandler); + + // Create the service and immediately close it + // (null ticketCache and authentications: the class-level createInstance stub calling the real + // method only matches isNull() for both) + final WinRMService winRMService = createInstance(endpointForFactoryTest, 30000L, null, null); + assertNotNull(winRMService); + winRMService.close(); + + // Verify that shutdown() was called on the factory for both the cmd and wql clients + verify(mockFactory, times(2)).shutdown(); + } }