From 7e1f3837d557502e8351c6c80652494adde84acd Mon Sep 17 00:00:00 2001 From: Ewerton Scaboro da Silva Date: Thu, 20 Aug 2026 14:52:44 -0700 Subject: [PATCH 1/5] Wait for the test proxies to bind, and stop ConnectionTests leaking clients Three test harness problems, salvaged from PR 1856. The SDK half of that PR was fixed independently in PR 1859, and the timeout change it proposed is superseded by PR 1862, but these three were never picked up. HttpProxyServer.startAsync only initiates the bind and returns a CompletionStage that completes once the port is listening. All four test classes that stand up a local proxy discarded it. Because the e2e tests run in parallel, tests can start sending traffic before the proxy is accepting, and get a connection refused from a proxy that is about to be perfectly healthy. ProxyServerTools.startProxyServer waits on that future, with a 30 second cap so a proxy that never binds fails the run with a clear error rather than hanging. ConnectionTests.ConnectionTestInstance.dispose was dead code. Nothing ever called it, so every test in the class leaked the client it opened along with its identity. Those clients keep retrying for the rest of the JVM's life, and the ones configured with proxy settings keep retrying through the proxies this class runs locally, competing with the tests still running. Once stopProxy closes those proxies they retry against a dead port instead, for the remainder of the job. This is the same leak that PR 1861 fixed in TokenRenewalTests, and it works against the parallelism cap added in PR 1862, since the whole point of that cap was to stop the proxies being starved. It is now called from an @After. While wiring that up, the ECC identities need care. Unlike every other identity in this class they are created by the test rather than taken from the shared pool, and they carry a self signed certificate that only this test knows about. disposeTestIdentity recycles rather than deletes when RECYCLE_TEST_IDENTITIES is set, and these are SELF_SIGNED, so recycling one would put a device with an unknown thumbprint into the x509 pool for a later test to fail on. They are deleted from the registry instead. ContractAPIMqttTest declared @Mocked Object mockSendLock and @Mocked Integer mockedInteger, and ContractAPIAmqpTest declared @Mocked Object mockSendLock. None of the three is referenced anywhere. Mocking java.lang.Object makes JMockit retransform it, which is a hazard to the whole JVM rather than to one test. To be clear about the evidence: this is not currently failing. The full provisioning suite passes on JDK 8 with reruns disabled, and no ContractAPI failure appears in the last seven CI builds. These are removed because they are unused and risky, not because they are breaking something today. Verified on JDK 8: iot-e2e-common test compilation succeeds, and provisioning-device-client runs 544 tests with no failures and no reruns, the same count as before the removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../sdk/iot/helpers/ProxyServerTools.java | 34 +++++++++++++ .../azure/sdk/iot/iothub/FileUploadTests.java | 5 +- .../iot/iothub/MultiplexingClientTests.java | 4 +- .../sdk/iot/iothub/TokenRenewalTests.java | 4 +- .../iothub/connection/ConnectionTests.java | 50 +++++++++++++++++-- .../contract/amqp/ContractAPIAmqpTest.java | 3 -- .../contract/mqtt/ContractAPIMqttTest.java | 6 --- 7 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/helpers/ProxyServerTools.java diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/helpers/ProxyServerTools.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/helpers/ProxyServerTools.java new file mode 100644 index 0000000000..ea7d656192 --- /dev/null +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/helpers/ProxyServerTools.java @@ -0,0 +1,34 @@ +/* + * Copyright (c) Microsoft. All rights reserved. + * Licensed under the MIT license. See LICENSE file in the project root for full license information. + */ + +package tests.integration.com.microsoft.azure.sdk.iot.helpers; + +import com.github.monkeywie.proxyee.server.HttpProxyServer; + +import java.util.concurrent.TimeUnit; + +/** + * Helpers for the local HTTP proxy servers that the proxy related integration tests run their traffic through. + */ +public class ProxyServerTools +{ + private static final int PROXY_START_TIMEOUT_SECONDS = 30; + + /** + * Start the provided proxy server on the provided port and block until it is actually listening on that port. + * + * {@link HttpProxyServer#startAsync(int)} only initiates the bind, so tests that don't wait on the returned future + * can start sending traffic to the proxy before it is listening. When that happens, the client under test gets a + * "Connection refused" instead of a working proxy. + * + * @param proxyServer the proxy server to start. + * @param port the port for the proxy server to listen on. + * @throws Exception if the proxy server could not be started within {@link #PROXY_START_TIMEOUT_SECONDS} seconds. + */ + public static void startProxyServer(HttpProxyServer proxyServer, int port) throws Exception + { + proxyServer.startAsync(port).toCompletableFuture().get(PROXY_START_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } +} diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/FileUploadTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/FileUploadTests.java index f313fe8f7b..f0ed962268 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/FileUploadTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/FileUploadTests.java @@ -36,6 +36,7 @@ import org.junit.runners.Parameterized; import tests.integration.com.microsoft.azure.sdk.iot.helpers.annotations.FlakeyTest; import tests.integration.com.microsoft.azure.sdk.iot.helpers.IntegrationTest; +import tests.integration.com.microsoft.azure.sdk.iot.helpers.ProxyServerTools; import tests.integration.com.microsoft.azure.sdk.iot.helpers.TestConstants; import tests.integration.com.microsoft.azure.sdk.iot.helpers.TestDeviceIdentity; import tests.integration.com.microsoft.azure.sdk.iot.helpers.Tools; @@ -147,12 +148,12 @@ public FileUploadState() } @BeforeClass - public static void startProxy() + public static void startProxy() throws Exception { HttpProxyServerConfig config = new HttpProxyServerConfig(); config.setHandleSsl(false); proxyServer = new HttpProxyServer().serverConfig(config); - proxyServer.startAsync(testProxyPort); + ProxyServerTools.startProxyServer(proxyServer, testProxyPort); } @AfterClass diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/MultiplexingClientTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/MultiplexingClientTests.java index d9889107e9..e892257cab 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/MultiplexingClientTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/MultiplexingClientTests.java @@ -203,12 +203,12 @@ public void tearDownTest() } @BeforeClass - public static void startProxy() + public static void startProxy() throws Exception { HttpProxyServerConfig config = new HttpProxyServerConfig(); config.setHandleSsl(false); proxyServer = new HttpProxyServer().serverConfig(config); - proxyServer.startAsync(testProxyPort); + ProxyServerTools.startProxyServer(proxyServer, testProxyPort); } @AfterClass diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java index e610ff07b9..7074532677 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java @@ -85,12 +85,12 @@ public static void setUp() } @BeforeClass - public static void startProxy() + public static void startProxy() throws Exception { HttpProxyServerConfig config = new HttpProxyServerConfig(); config.setHandleSsl(false); proxyServer = new HttpProxyServer().serverConfig(config); - proxyServer.startAsync(testProxyPort); + ProxyServerTools.startProxyServer(proxyServer, testProxyPort); } @AfterClass diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java index ea8ec9ed85..e7b1a8efe8 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java @@ -12,6 +12,7 @@ import com.microsoft.azure.sdk.iot.service.registry.Module; import com.microsoft.azure.sdk.iot.service.registry.RegistryClient; import lombok.extern.slf4j.Slf4j; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -23,6 +24,7 @@ import tests.integration.com.microsoft.azure.sdk.iot.helpers.annotations.StandardTierHubOnlyTest; import javax.net.ssl.SSLContext; +import java.io.IOException; import java.net.InetSocketAddress; import java.net.Proxy; import java.util.*; @@ -97,6 +99,10 @@ public class ConnectionTestInstance { public IotHubClientProtocol protocol; public TestIdentity identity; + + // ECC identities are created by this test rather than taken from the shared pool, and they carry a + // certificate that only this test knows about, so they must never be recycled back into that pool. + private boolean identityIsEccIdentity; public AuthenticationType authenticationType; public ClientType clientType; public boolean useHttpProxy; @@ -150,6 +156,7 @@ public void setupEccDevice() throws Exception X509CertificateGenerator certificateGenerator = new X509CertificateGenerator(X509CertificateGenerator.CertificateAlgorithm.ECC); SSLContext sslContext = SSLContextBuilder.buildSSLContext(certificateGenerator.getX509Certificate(), certificateGenerator.getPrivateKey()); optionsBuilder.sslContext(sslContext); + this.identityIsEccIdentity = true; if (clientType == ClientType.DEVICE_CLIENT) { @@ -183,12 +190,35 @@ else if (clientType == ClientType.MODULE_CLIENT) public void dispose() { - if (this.identity != null && this.identity.getClient() != null) + if (this.identity == null) + { + return; + } + + if (this.identity.getClient() != null) { this.identity.getClient().close(); } - Tools.disposeTestIdentity(this.identity, iotHubConnectionString); + if (this.identityIsEccIdentity) + { + // Recycling this identity would hand a device carrying a certificate that no other test knows about to + // the next test that takes an x509 identity from the shared pool, so delete it instead. + try + { + Tools.getRegistyManager(iotHubConnectionString).removeDevice(this.identity.getDeviceId()); + } + catch (IOException | IotHubException e) + { + log.error("Failed to clean up ECC test device {}", this.identity.getDeviceId(), e); + } + } + else + { + Tools.disposeTestIdentity(this.identity, iotHubConnectionString); + } + + this.identity = null; } } @@ -208,18 +238,28 @@ public void dispose() protected static final String testProxyPass = "1234"; // lgtm @BeforeClass - public static void startProxy() + public static void startProxy() throws Exception { HttpProxyServerConfig config = new HttpProxyServerConfig(); config.setAuthenticationProvider(new BasicProxyAuthenticator(testProxyUser, testProxyPass)); config.setHandleSsl(false); proxyServer = new HttpProxyServer().serverConfig(config); - proxyServer.startAsync(testProxyPort); + ProxyServerTools.startProxyServer(proxyServer, testProxyPort); HttpProxyServerConfig configWithoutAuth = new HttpProxyServerConfig(); configWithoutAuth.setHandleSsl(false); proxyServerWithoutAuth = new HttpProxyServer().serverConfig(configWithoutAuth); - proxyServerWithoutAuth.startAsync(testProxyPortWithoutAuth); + ProxyServerTools.startProxyServer(proxyServerWithoutAuth, testProxyPortWithoutAuth); + } + + // Without this, every test in this class leaks the client it opened. Those clients keep retrying their + // connections for the rest of the JVM's life, and the ones configured with proxy settings keep retrying through + // the proxies this class runs locally, which competes with the tests that are still running. Once stopProxy has + // closed those proxies they retry against a dead port instead, for the remainder of the job. + @After + public void disposeTestInstance() + { + testInstance.dispose(); } @AfterClass diff --git a/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/amqp/ContractAPIAmqpTest.java b/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/amqp/ContractAPIAmqpTest.java index ef4f44bc06..b02cd55070 100644 --- a/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/amqp/ContractAPIAmqpTest.java +++ b/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/amqp/ContractAPIAmqpTest.java @@ -67,9 +67,6 @@ public class ContractAPIAmqpTest @Mocked Map mockedHashMap; - @Mocked - Object mockSendLock; - @Mocked ProvisioningDeviceClientConfig mockedProvisioningDeviceClientConfig; diff --git a/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/mqtt/ContractAPIMqttTest.java b/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/mqtt/ContractAPIMqttTest.java index e8498ea2d9..e56b77db88 100644 --- a/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/mqtt/ContractAPIMqttTest.java +++ b/provisioning/provisioning-device-client/src/test/java/com/microsoft/azure/sdk/iot/provisioning/device/internal/contract/mqtt/ContractAPIMqttTest.java @@ -73,18 +73,12 @@ public class ContractAPIMqttTest @Mocked MqttMessage mockedMqttMessage; - @Mocked - Object mockSendLock; - @Mocked ProvisioningDeviceClientConfig mockedProvisioningDeviceClientConfig; @Mocked DeviceRegistrationParser mockedDeviceRegistrationParser; - @Mocked - Integer mockedInteger; - @Mocked byte[] mockedByteArray = new byte[10]; From bf7560ea9d107dd3079460f59f947359170335ba Mon Sep 17 00:00:00 2001 From: Ewerton Scaboro da Silva Date: Fri, 21 Aug 2026 13:13:18 -0700 Subject: [PATCH 2/5] Send the ECC tests through the proxy their parameters ask for CanOpenConnectionWithECCCertificates[MQTT_WS_SELF_SIGNED_MODULE_CLIENT_true_false] and its true_true counterpart are the only tests that have failed on the Java Windows nightly in the last 15 main builds. They fail intermittently, roughly a third of runs, and always the same way: the registry work finishes in about 140 milliseconds and then there is 60 seconds of complete silence with no connection status transition at all before the JUnit timeout fires. setupEccDevice ignored useHttpProxyAuth. setup, which every other test in this class uses, picks between the authenticated proxy on 8899 and the unauthenticated one on 9000 based on that flag. setupEccDevice had only the first branch, so every proxied ECC variant went to the authenticated proxy. Two consequences. Both proxied ECC variants piled onto one embedded proxy while the other sat idle, doubling the demand on a server that runs inside the same JVM as the tests. And the true_false variant never tested what its name says: it claims to cover ECC certificates through a proxy that does not require authentication, and it actually exercised the authenticated one, so that combination had no coverage at all. The two tests that fail are exactly the two that were misrouted. Rather than adding the missing branch to the second copy, both call sites now share applyProxySettings. Having the same decision written out twice is what allowed them to drift, and the copy that drifted was the one used by the test that fails. CanOpenMultiplexingConnection keeps its own copy because it builds MultiplexingClientOptions rather than ClientOptions, so it cannot share the helper. This should reduce the failure rate rather than being guaranteed to eliminate it. The underlying condition is contention for the embedded proxies, and this removes one contributor to it. Even on a passing Windows run these tests take about 11.4 seconds against a 60 second budget, so the headroom is smaller than the pass result suggests. Tracked by work item 39365585. Verified with mvn -pl iot-e2e-tests/common -am test-compile on JDK 8. The behaviour of setup() is unchanged, so the only functional difference is which proxy the ECC variants use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../iothub/connection/ConnectionTests.java | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java index e7b1a8efe8..f03060407e 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java @@ -117,22 +117,37 @@ public ConnectionTestInstance(IotHubClientProtocol protocol, AuthenticationType this.useHttpProxyAuth = useHttpProxyAuth; } - public void setup() throws Exception + /** + * Configure this test's proxy settings on the given builder, if this variant uses a proxy at all. + * + *

Both setup paths go through here so that they cannot drift apart. They previously had separate copies of + * this logic, and the copy in setupEccDevice was missing the unauthenticated branch entirely.

+ * + * @param optionsBuilder The builder to apply the proxy settings to + */ + private void applyProxySettings(ClientOptions.ClientOptionsBuilder optionsBuilder) { - ClientOptions.ClientOptionsBuilder optionsBuilder = ClientOptions.builder(); - if (this.useHttpProxy) + if (!this.useHttpProxy) { - if (this.useHttpProxyAuth) - { - Proxy testProxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(testProxyHostname, testProxyPort)); - optionsBuilder.proxySettings(new ProxySettings(testProxy, testProxyUser, testProxyPass.toCharArray())); - } - else - { - Proxy testProxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(testProxyHostnameWithoutAuth, testProxyPortWithoutAuth)); - optionsBuilder.proxySettings(new ProxySettings(testProxy)); - } + return; + } + + if (this.useHttpProxyAuth) + { + Proxy testProxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(testProxyHostname, testProxyPort)); + optionsBuilder.proxySettings(new ProxySettings(testProxy, testProxyUser, testProxyPass.toCharArray())); + } + else + { + Proxy testProxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(testProxyHostnameWithoutAuth, testProxyPortWithoutAuth)); + optionsBuilder.proxySettings(new ProxySettings(testProxy)); } + } + + public void setup() throws Exception + { + ClientOptions.ClientOptionsBuilder optionsBuilder = ClientOptions.builder(); + applyProxySettings(optionsBuilder); if (clientType == ClientType.DEVICE_CLIENT) { @@ -147,11 +162,7 @@ else if (clientType == ClientType.MODULE_CLIENT) public void setupEccDevice() throws Exception { ClientOptions.ClientOptionsBuilder optionsBuilder = ClientOptions.builder(); - if (this.useHttpProxy) - { - Proxy testProxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(testProxyHostname, testProxyPort)); - optionsBuilder.proxySettings(new ProxySettings(testProxy, testProxyUser, testProxyPass.toCharArray())); - } + applyProxySettings(optionsBuilder); X509CertificateGenerator certificateGenerator = new X509CertificateGenerator(X509CertificateGenerator.CertificateAlgorithm.ECC); SSLContext sslContext = SSLContextBuilder.buildSSLContext(certificateGenerator.getX509Certificate(), certificateGenerator.getPrivateKey()); From 7141d3cdc9bfc15dedc8818b9a187d65ad0ce727 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 07:10:53 +0000 Subject: [PATCH 3/5] Clean up ECC devices that were registered before setup failed setupEccDevice registers the ECC device before it has anything to assign to identity, and for module variants it then registers a module and constructs a client, either of which can throw. When that happened the new @After reached dispose() with identity still null, took the early return, and left the device behind in the registry - the same leak the rest of this change is removing. Record the device id as soon as the registration succeeds and drive the cleanup off that instead of off identity, so a half provisioned ECC identity is still deleted. Deleting the device deletes its module too, so the module needs no separate tracking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../iothub/connection/ConnectionTests.java | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java index f03060407e..988a37e9f5 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java @@ -101,8 +101,11 @@ public class ConnectionTestInstance public TestIdentity identity; // ECC identities are created by this test rather than taken from the shared pool, and they carry a - // certificate that only this test knows about, so they must never be recycled back into that pool. - private boolean identityIsEccIdentity; + // certificate that only this test knows about, so they must never be recycled back into that pool. This is + // recorded as soon as the device is registered, rather than being derived from the identity, because the + // rest of setupEccDevice can fail after that registration has already happened. Deleting the device also + // deletes any module underneath it, so the module does not need to be tracked separately. + private String eccDeviceIdToDelete; public AuthenticationType authenticationType; public ClientType clientType; public boolean useHttpProxy; @@ -167,7 +170,6 @@ public void setupEccDevice() throws Exception X509CertificateGenerator certificateGenerator = new X509CertificateGenerator(X509CertificateGenerator.CertificateAlgorithm.ECC); SSLContext sslContext = SSLContextBuilder.buildSSLContext(certificateGenerator.getX509Certificate(), certificateGenerator.getPrivateKey()); optionsBuilder.sslContext(sslContext); - this.identityIsEccIdentity = true; if (clientType == ClientType.DEVICE_CLIENT) { @@ -175,6 +177,7 @@ public void setupEccDevice() throws Exception eccDevice.setThumbprint(certificateGenerator.getX509Thumbprint(), certificateGenerator.getX509Thumbprint()); Tools.addDeviceWithRetry(new RegistryClient(iotHubConnectionString), eccDevice); + this.eccDeviceIdToDelete = eccDevice.getDeviceId(); String deviceConnectionString = Tools.getDeviceConnectionString(iotHubConnectionString, eccDevice); this.identity = new TestDeviceIdentity( @@ -189,6 +192,8 @@ else if (clientType == ClientType.MODULE_CLIENT) eccModule.setThumbprint(certificateGenerator.getX509Thumbprint(), certificateGenerator.getX509Thumbprint()); Tools.addDeviceWithRetry(new RegistryClient(iotHubConnectionString), eccDevice); + this.eccDeviceIdToDelete = eccDevice.getDeviceId(); + Tools.addModuleWithRetry(new RegistryClient(iotHubConnectionString), eccModule); String moduleConnectionString = Tools.getDeviceConnectionString(iotHubConnectionString, eccDevice) + ";ModuleId=" + eccModule.getId(); @@ -201,30 +206,29 @@ else if (clientType == ClientType.MODULE_CLIENT) public void dispose() { - if (this.identity == null) - { - return; - } - - if (this.identity.getClient() != null) + if (this.identity != null && this.identity.getClient() != null) { this.identity.getClient().close(); } - if (this.identityIsEccIdentity) + if (this.eccDeviceIdToDelete != null) { // Recycling this identity would hand a device carrying a certificate that no other test knows about to - // the next test that takes an x509 identity from the shared pool, so delete it instead. + // the next test that takes an x509 identity from the shared pool, so delete it instead. This runs even + // when the identity was never finished being built, because the device is in the registry from the + // moment it is registered, whether or not the rest of the setup succeeded. try { - Tools.getRegistyManager(iotHubConnectionString).removeDevice(this.identity.getDeviceId()); + Tools.getRegistyManager(iotHubConnectionString).removeDevice(this.eccDeviceIdToDelete); } catch (IOException | IotHubException e) { - log.error("Failed to clean up ECC test device {}", this.identity.getDeviceId(), e); + log.error("Failed to clean up ECC test device {}", this.eccDeviceIdToDelete, e); } + + this.eccDeviceIdToDelete = null; } - else + else if (this.identity != null) { Tools.disposeTestIdentity(this.identity, iotHubConnectionString); } From 2fa541cfc94b5948a28c62994ee3731ea9b76f9d Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:43:09 +0000 Subject: [PATCH 4/5] Stop the @After from nulling the identity a timed out test is still using Build 162413 failed on Linux JDK 17 and JDK 21 with NullPointerException: Cannot invoke TestIdentity.getClient() because this.testInstance.identity is null at ConnectionTests.CanOpenConnection(ConnectionTests.java:336) Line 336 is the closing client.close(). Every test in this class is bounded by @Test(timeout = 60000), and JUnit runs the method body on a separate thread that it abandons, still running, when the timeout fires. @After is outside that timeout, so the dispose() added by this change ran on the main thread while the abandoned thread was still partway through the test body, and the identity = null in dispose() pulled the field out from under it. The four other classes that dispose from an @After do not clear the field, and none of them bound their tests with a timeout. Clearing it was hygiene rather than a requirement - setup() assigns the field on every attempt - so it is dropped, which restores the existing convention. The two timeout bounded test bodies now also take the client once into a local rather than re-reading it off the shared instance for each call, so they no longer depend on that field surviving the test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../iothub/connection/ConnectionTests.java | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java index 988a37e9f5..2497cf69d8 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java @@ -233,7 +233,11 @@ else if (this.identity != null) Tools.disposeTestIdentity(this.identity, iotHubConnectionString); } - this.identity = null; + // identity is deliberately left set. Every test in this class is bounded by @Test(timeout = 60000), and + // JUnit runs that method on a separate thread which it abandons, still running, when the timeout fires. + // @After is outside the timeout, so dispose() executes while that abandoned thread is still working its + // way through the rest of the test body. Clearing the field here made those threads dereference null. + // The other test classes that call dispose() from an @After do not clear it either. } } @@ -328,16 +332,21 @@ private static void logConnectionStatusChanges(InternalClient client) public void CanOpenConnection() throws Exception { testInstance.setup(); - logConnectionStatusChanges(testInstance.identity.getClient()); - testInstance.identity.getClient().open(true); + + // Held locally rather than read back off testInstance for each call. The @After that disposes this instance + // runs outside this method's timeout, so it can execute while this thread is still here after a timeout. + InternalClient client = testInstance.identity.getClient(); + + logConnectionStatusChanges(client); + client.open(true); // deviceClient.open() is a no-op on HTTP, so a message needs to be sent to actually test opening the connection if (testInstance.protocol == HTTPS) { - testInstance.identity.getClient().sendEvent(new Message("some message")); + client.sendEvent(new Message("some message")); } - testInstance.identity.getClient().close(); + client.close(); } @IotHubTest @@ -358,16 +367,18 @@ public void CanOpenConnectionWithECCCertificates() throws Exception testInstance.setupEccDevice(); - logConnectionStatusChanges(testInstance.identity.getClient()); - testInstance.identity.getClient().open(true); + InternalClient client = testInstance.identity.getClient(); + + logConnectionStatusChanges(client); + client.open(true); // deviceClient.open() is a no-op on HTTP, so a message needs to be sent to actually test opening the connection if (testInstance.protocol == HTTPS) { - testInstance.identity.getClient().sendEvent(new Message("some message")); + client.sendEvent(new Message("some message")); } - testInstance.identity.getClient().close(); + client.close(); } @Test(timeout = 60000) // 1 minute From 1c1eee25fb3cc940ea7ecbe8bc3ed347a97341a9 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:01:11 +0000 Subject: [PATCH 5/5] Close what teardown could still miss in ConnectionTests Two gaps in the teardown this change introduced, both leaving clients retrying for the life of the JVM - the leak this class is trying to stop. 1. Identities produced after teardown had already run @After sits outside the @Test(timeout = 60000) wrapper, so it can execute while the thread JUnit abandoned at the timeout is still inside setup(). The identity that thread went on to acquire had no owner and leaked. Teardown now takes ownership of the tracked fields and clears them, which makes it idempotent, and each setup path re-checks afterwards and disposes what it produced if teardown had already run. Nothing blocks in either direction: waiting on a setup that is itself hung, which is how these tests have actually timed out, would stall the rest of the run. The public identity field is still never cleared, so an abandoned thread can keep reading it. Cleanup ownership moved to a separate field so that clearing it cannot resurrect that NPE, and so a second teardown cannot requeue the same identity into the shared pool twice. 2. CanOpenMultiplexingConnection kept its clients in locals Its MultiplexingClient and the three DeviceClients registered to it are local to the method, so the new @After never saw them, and close() sat after open() rather than in the finally. A failed or timed out open skipped it and left all four retrying. close() moved into the finally, guarded so it cannot mask what the test threw. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../iothub/connection/ConnectionTests.java | 143 +++++++++++++++--- 1 file changed, 119 insertions(+), 24 deletions(-) diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java index 2497cf69d8..2b3222c1fc 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/connection/ConnectionTests.java @@ -106,6 +106,17 @@ public class ConnectionTestInstance // rest of setupEccDevice can fail after that registration has already happened. Deleting the device also // deletes any module underneath it, so the module does not need to be tracked separately. private String eccDeviceIdToDelete; + + // What teardown still owns, kept separate from the public identity field above. dispose() clears these but + // deliberately leaves identity set, because a test thread abandoned by the JUnit timeout may still read it. + // Clearing them is what makes dispose() safe to run more than once: without it a second run would close the + // same client twice and, worse, requeue the same identity into the shared pool twice. + private TestIdentity identityToDispose; + + // Set once teardown has run. Guarded by lifecycleLock along with the two fields above. + private boolean disposed; + + private final Object lifecycleLock = new Object(); public AuthenticationType authenticationType; public ClientType clientType; public boolean useHttpProxy; @@ -154,12 +165,14 @@ public void setup() throws Exception if (clientType == ClientType.DEVICE_CLIENT) { - this.identity = Tools.getTestDevice(iotHubConnectionString, this.protocol, this.authenticationType, false, optionsBuilder); + trackForCleanup(Tools.getTestDevice(iotHubConnectionString, this.protocol, this.authenticationType, false, optionsBuilder)); } else if (clientType == ClientType.MODULE_CLIENT) { - this.identity = Tools.getTestModule(iotHubConnectionString, this.protocol, this.authenticationType , false, optionsBuilder); + trackForCleanup(Tools.getTestModule(iotHubConnectionString, this.protocol, this.authenticationType , false, optionsBuilder)); } + + disposeIfTeardownAlreadyRan(); } public void setupEccDevice() throws Exception @@ -177,12 +190,12 @@ public void setupEccDevice() throws Exception eccDevice.setThumbprint(certificateGenerator.getX509Thumbprint(), certificateGenerator.getX509Thumbprint()); Tools.addDeviceWithRetry(new RegistryClient(iotHubConnectionString), eccDevice); - this.eccDeviceIdToDelete = eccDevice.getDeviceId(); + trackEccDeviceForCleanup(eccDevice.getDeviceId()); String deviceConnectionString = Tools.getDeviceConnectionString(iotHubConnectionString, eccDevice); - this.identity = new TestDeviceIdentity( + trackForCleanup(new TestDeviceIdentity( new DeviceClient(deviceConnectionString, testInstance.protocol, optionsBuilder.build()), - eccDevice); + eccDevice)); } else if (clientType == ClientType.MODULE_CLIENT) { @@ -192,26 +205,104 @@ else if (clientType == ClientType.MODULE_CLIENT) eccModule.setThumbprint(certificateGenerator.getX509Thumbprint(), certificateGenerator.getX509Thumbprint()); Tools.addDeviceWithRetry(new RegistryClient(iotHubConnectionString), eccDevice); - this.eccDeviceIdToDelete = eccDevice.getDeviceId(); + trackEccDeviceForCleanup(eccDevice.getDeviceId()); Tools.addModuleWithRetry(new RegistryClient(iotHubConnectionString), eccModule); String moduleConnectionString = Tools.getDeviceConnectionString(iotHubConnectionString, eccDevice) + ";ModuleId=" + eccModule.getId(); - this.identity = new TestModuleIdentity( + trackForCleanup(new TestModuleIdentity( new ModuleClient(moduleConnectionString, testInstance.protocol, optionsBuilder.build()), eccDevice, - eccModule); + eccModule)); + } + + disposeIfTeardownAlreadyRan(); + } + + /** + * Hand an identity to teardown, and publish it for the test body to use. + * + * @param newIdentity The identity this test just acquired or created + */ + private void trackForCleanup(TestIdentity newIdentity) + { + // Published for the test body. Never cleared, so a thread the JUnit timeout abandoned can keep reading it. + this.identity = newIdentity; + + synchronized (lifecycleLock) + { + this.identityToDispose = newIdentity; + } + } + + /** + * Hand a freshly registered ECC device to teardown, so it is removed from the registry even if the rest of + * setupEccDevice never completes. + * + * @param deviceId The device id that was just added to the registry + */ + private void trackEccDeviceForCleanup(String deviceId) + { + synchronized (lifecycleLock) + { + this.eccDeviceIdToDelete = deviceId; + } + } + + /** + * Dispose anything registered after teardown already ran. + * + *

Every test in this class is bounded by {@code @Test(timeout = 60000)}. JUnit runs the test body on a + * separate thread and, when the timeout fires, abandons that thread while it is still running. {@code @After} + * is outside the timeout, so teardown can execute while setup on the abandoned thread has not finished + * acquiring its identity. Without this, the identity that setup goes on to produce would have no owner and + * would leak, which is precisely the leak this class is trying to stop.

+ * + *

This does not wait for setup, in either direction. Blocking teardown on a setup that is itself hung - + * which is how these tests have actually timed out - would stall the rest of the run.

+ */ + private void disposeIfTeardownAlreadyRan() + { + boolean teardownAlreadyRan; + synchronized (lifecycleLock) + { + teardownAlreadyRan = this.disposed; + } + + if (teardownAlreadyRan) + { + dispose(); } } + /** + * Close and dispose whatever this instance currently owns. + * + *

Safe to call more than once, and safe to call concurrently with setup: it takes ownership of the tracked + * fields and clears them, so a second call finds only what was registered since the first.

+ */ public void dispose() { - if (this.identity != null && this.identity.getClient() != null) + TestIdentity identityToClean; + String eccDeviceIdToClean; + + synchronized (lifecycleLock) + { + this.disposed = true; + + identityToClean = this.identityToDispose; + eccDeviceIdToClean = this.eccDeviceIdToDelete; + + this.identityToDispose = null; + this.eccDeviceIdToDelete = null; + } + + if (identityToClean != null && identityToClean.getClient() != null) { - this.identity.getClient().close(); + identityToClean.getClient().close(); } - if (this.eccDeviceIdToDelete != null) + if (eccDeviceIdToClean != null) { // Recycling this identity would hand a device carrying a certificate that no other test knows about to // the next test that takes an x509 identity from the shared pool, so delete it instead. This runs even @@ -219,25 +310,17 @@ public void dispose() // moment it is registered, whether or not the rest of the setup succeeded. try { - Tools.getRegistyManager(iotHubConnectionString).removeDevice(this.eccDeviceIdToDelete); + Tools.getRegistyManager(iotHubConnectionString).removeDevice(eccDeviceIdToClean); } catch (IOException | IotHubException e) { - log.error("Failed to clean up ECC test device {}", this.eccDeviceIdToDelete, e); + log.error("Failed to clean up ECC test device {}", eccDeviceIdToClean, e); } - - this.eccDeviceIdToDelete = null; } - else if (this.identity != null) + else if (identityToClean != null) { - Tools.disposeTestIdentity(this.identity, iotHubConnectionString); + Tools.disposeTestIdentity(identityToClean, iotHubConnectionString); } - - // identity is deliberately left set. Every test in this class is bounded by @Test(timeout = 60000), and - // JUnit runs that method on a separate thread which it abandons, still running, when the timeout fires. - // @After is outside the timeout, so dispose() executes while that abandoned thread is still working its - // way through the rest of the test body. Clearing the field here made those threads dereference null. - // The other test classes that call dispose() from an @After do not clear it either. } } @@ -426,10 +509,22 @@ public void CanOpenMultiplexingConnection() throws Exception multiplexingClient.registerDeviceClients(testClients); multiplexingClient.open(true); - multiplexingClient.close(); } finally { + // Closing here rather than after open() so that a failed or timed out open still gives the client and the + // three device clients registered to it back. Otherwise they keep retrying for the life of the JVM, and + // the proxied variants keep retrying through the proxies this class runs locally. + try + { + multiplexingClient.close(); + } + catch (Exception e) + { + // Swallowed so it cannot mask whatever the test itself threw. + log.error("Failed to close the multiplexing client", e); + } + Tools.disposeTestIdentities(testIdentities, iotHubConnectionString); } }