From fe3c7c6a1ba61848642dec848889a3f5ff7322c5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 06:03:05 +0000 Subject: [PATCH 1/3] Initial plan From 09978f87faafd31807eade85578ac046426d8ec8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 06:14:54 +0000 Subject: [PATCH 2/3] Fix thread leak: shutdown AsyncHttpEncryptionAwareConduitFactory when closing WinRMService Agent-Logs-Url: https://github.com/MetricsHub/winrm-java/sessions/266b5281-d5d0-456c-9f38-84648d52a6c0 Co-authored-by: bertysentry <32521698+bertysentry@users.noreply.github.com> --- .../winrm/service/WinRMService.java | 17 +++++ .../client/WinRMInvocationHandler.java | 8 ++ .../winrm/service/WinRMServiceTest.java | 74 +++++++++++++++++++ 3 files changed, 99 insertions(+) diff --git a/src/main/java/org/metricshub/winrm/service/WinRMService.java b/src/main/java/org/metricshub/winrm/service/WinRMService.java index 1df15c6..d1426aa 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 static 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..043caa7 100644 --- a/src/main/java/org/metricshub/winrm/service/client/WinRMInvocationHandler.java +++ b/src/main/java/org/metricshub/winrm/service/client/WinRMInvocationHandler.java @@ -403,6 +403,14 @@ static Client getWebServiceClient( client.getOutInterceptors().add(new SignAndEncryptOutInterceptor()); // this is different to endpoint properties + // Shutdown any existing factory before replacing to prevent thread leaks during authentication retries + final Object existingFactory = client + .getEndpoint() + .getEndpointInfo() + .getProperty(HTTPConduitFactory.class.getName()); + if (existingFactory instanceof AsyncHttpEncryptionAwareConduitFactory) { + ((AsyncHttpEncryptionAwareConduitFactory) existingFactory).shutdown(); + } client .getEndpoint() .getEndpointInfo() diff --git a/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java b/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java index 27488c7..5c72445 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,73 @@ 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 + final WinRMService winRMService = createInstance(endpointForFactoryTest, 30000L, null, singletonList(NTLM)); + winRMService.close(); + + // Verify that shutdown() was called on the factory for both the cmd and wql clients + verify(mockFactory, times(2)).shutdown(); + } } From 08e8b603affe65969e130e76f3a6dc55485c4d6e Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Tue, 21 Jul 2026 13:23:17 +0200 Subject: [PATCH 3/3] Fix conduit factory lifecycle on authentication retries - getWebServiceClient: register the AsyncHttpEncryptionAwareConduitFactory only once instead of shutting down and replacing it on every call. The client's cached conduit keeps using the factory it was created with, so replacing the endpoint property only orphaned factory instances (which hold no threads until a conduit uses them), and shutting down the in-use factory made AsyncHTTPConduit.setupConnection silently fall back to the synchronous transport on the retry request. Reusing the factory also guarantees WinRMService.close() shuts down the instance that owns the background threads. - WinRMService.shutdownConduitFactory: make it an instance method so the static mock of WinRMService in tests cannot swallow it. - WinRMServiceTest: pass null ticketCache/authentications so the call matches the class-level createInstance stub that calls the real method (the any(Path.class) matcher does not match null, so the mocked createInstance returned null and the test failed with an NPE). Co-Authored-By: Claude Fable 5 --- .../winrm/service/WinRMService.java | 2 +- .../client/WinRMInvocationHandler.java | 23 ++++++++++--------- .../winrm/service/WinRMServiceTest.java | 5 +++- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/service/WinRMService.java b/src/main/java/org/metricshub/winrm/service/WinRMService.java index d1426aa..6819c3c 100644 --- a/src/main/java/org/metricshub/winrm/service/WinRMService.java +++ b/src/main/java/org/metricshub/winrm/service/WinRMService.java @@ -375,7 +375,7 @@ public void close() { * * @param client the CXF {@link Client} whose conduit factory should be shut down */ - private static void shutdownConduitFactory(final Client client) { + private void shutdownConduitFactory(final Client client) { final Object factory = client.getEndpoint().getEndpointInfo().getProperty(HTTPConduitFactory.class.getName()); if (factory instanceof AsyncHTTPConduitFactory) { ((AsyncHTTPConduitFactory) factory).shutdown(); 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 043caa7..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,18 +404,18 @@ static Client getWebServiceClient( client.getOutInterceptors().add(new SignAndEncryptOutInterceptor()); // this is different to endpoint properties - // Shutdown any existing factory before replacing to prevent thread leaks during authentication retries - final Object existingFactory = client - .getEndpoint() - .getEndpointInfo() - .getProperty(HTTPConduitFactory.class.getName()); - if (existingFactory instanceof AsyncHttpEncryptionAwareConduitFactory) { - ((AsyncHttpEncryptionAwareConduitFactory) existingFactory).shutdown(); + // 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()); } - client - .getEndpoint() - .getEndpointInfo() - .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 5c72445..543056a 100644 --- a/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java +++ b/src/test/java/org/metricshub/winrm/service/WinRMServiceTest.java @@ -336,7 +336,10 @@ void testCloseShutdownsConduitFactories() throws Exception { .thenReturn(wqlHandler); // Create the service and immediately close it - final WinRMService winRMService = createInstance(endpointForFactoryTest, 30000L, null, singletonList(NTLM)); + // (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