From 74dde8e13a3185f90ec4a8180b226cc344f834b9 Mon Sep 17 00:00:00 2001 From: Ewerton Scaboro da Silva Date: Wed, 19 Aug 2026 09:57:24 -0700 Subject: [PATCH 1/2] Stop tokenRenewalWorks from leaking clients when it fails tokenRenewalWorks opens around 17 device and module clients plus two multiplexing clients, then closes them and disposes of their identities at the end of the happy path. Nothing cleaned up when the test failed part way through, so every client that had already been opened was left running. Those clients do not go idle. They keep retrying their connections for the rest of the JVM's life, and the three of them that are given proxy settings keep retrying through the proxyee server this class runs on 127.0.0.1:8898. The test is wrapped in RerunFailedTestRule, so a failure is retried twice more in the same JVM, and each attempt leaves another set of clients behind. The reruns end up competing with the leaked clients from the earlier attempts for the agent's CPU and for the local proxy. That is what happened on the JDK 11 job of build 161991: 22:38:14.104 the last client opens successfully, a plain MQTT_WS device 22:38:14.104 the next client in creation order, the MQTT_WS device that goes through the local proxy, starts opening ... 60 seconds of no output at all 22:39:14.105 attempt 1 fails, MqttException: Timed out waiting for a response from the server, which is Mqtt.CONNECTION_TIMEOUT expiring while waiting for a CONNACK 22:40:20.071 attempt 2 fails the same way, 60 seconds again 22:41:24.043 attempt 3 fails the same way, 60 seconds again Each attempt failed in exactly the same place after exactly 60 seconds, which is what an agent that cannot service the proxied connection in time looks like, rather than three independent flakes. Wraps the body of the test in a try/finally that closes the clients, closes the multiplexing clients, and disposes of the identities. Cleanup is best effort and never throws, so a client that cannot be closed does not replace the failure the test is already reporting. The successful path is left as it is, because the connectivity assertions have to run after the clients have been closed, which makes the cleanup in the finally a no-op when the test passes. Verified with mvn -pl iot-e2e-tests/common -am test-compile on JDK 8. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../sdk/iot/iothub/TokenRenewalTests.java | 167 +++++++++++++----- 1 file changed, 121 insertions(+), 46 deletions(-) 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 289ad24b22..a24bd38b58 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 @@ -120,63 +120,138 @@ public void tokenRenewalWorks() throws Exception ArrayList amqpwsMultiplexedClients = new ArrayList<>(); MultiplexingClient amqpWsMultiplexingClient = createMultiplexedClientToTest(AMQPS_WS, amqpwsMultiplexedClients, hostname); - // Allow registry operations some buffer time before attempting to open connections for them - Thread.sleep(2000); + try + { + // Allow registry operations some buffer time before attempting to open connections for them + Thread.sleep(2000); - //service grants a 10 minute grace period beyond when sas token expires, this test attempts to send a message after that grace period - // to ensure that the first sas token has expired, and that the sas token was renewed successfully. - final long WAIT_BUFFER_FOR_TOKEN_TO_EXPIRE = EXPIRED_SAS_TOKEN_GRACE_PERIOD_SECONDS + EXTRA_BUFFER_TO_ENSURE_TOKEN_EXPIRED_SECONDS; + //service grants a 10 minute grace period beyond when sas token expires, this test attempts to send a message after that grace period + // to ensure that the first sas token has expired, and that the sas token was renewed successfully. + final long WAIT_BUFFER_FOR_TOKEN_TO_EXPIRE = EXPIRED_SAS_TOKEN_GRACE_PERIOD_SECONDS + EXTRA_BUFFER_TO_ENSURE_TOKEN_EXPIRED_SECONDS; - clients.addAll(amqpMultiplexedClients); - clients.addAll(amqpwsMultiplexedClients); + clients.addAll(amqpMultiplexedClients); + clients.addAll(amqpwsMultiplexedClients); - Success[] amqpDisconnectDidNotHappenSuccesses = new Success[clients.size()]; - Success[] mqttDisconnectDidHappenSuccesses = new Success[clients.size()]; - Success[] shutdownWasGracefulSuccesses = new Success[clients.size()]; - Success[] mqttDisconnectHadTokenExpiredReasonSuccesses = new Success[clients.size()]; - for (int clientIndex = 0; clientIndex < clients.size(); clientIndex++) - { - amqpDisconnectDidNotHappenSuccesses[clientIndex] = new Success(); - mqttDisconnectDidHappenSuccesses[clientIndex] = new Success(); - shutdownWasGracefulSuccesses[clientIndex] = new Success(); - mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex] = new Success(); + Success[] amqpDisconnectDidNotHappenSuccesses = new Success[clients.size()]; + Success[] mqttDisconnectDidHappenSuccesses = new Success[clients.size()]; + Success[] shutdownWasGracefulSuccesses = new Success[clients.size()]; + Success[] mqttDisconnectHadTokenExpiredReasonSuccesses = new Success[clients.size()]; + for (int clientIndex = 0; clientIndex < clients.size(); clientIndex++) + { + amqpDisconnectDidNotHappenSuccesses[clientIndex] = new Success(); + mqttDisconnectDidHappenSuccesses[clientIndex] = new Success(); + shutdownWasGracefulSuccesses[clientIndex] = new Success(); + mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex] = new Success(); - amqpDisconnectDidNotHappenSuccesses[clientIndex].setResult(true); //assume success until unexpected DISCONNECTED_RETRYING - mqttDisconnectDidHappenSuccesses[clientIndex].setResult(false); //assume failure until DISCONNECTED_RETRYING is triggered by token expiring - shutdownWasGracefulSuccesses[clientIndex].setResult(true); //assume success until DISCONNECTED callback without CLIENT_CLOSE - - mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex].setResult(false); //assume failure until first disconnected_retrying executes with reason EXPIRED_SAS_TOKEN - - clients.get(clientIndex).setConnectionStatusChangeCallback( - new IotHubConnectionStatusChangeTokenRenewalCallbackVerifier( - clients.get(clientIndex).getConfig().getProtocol(), - amqpDisconnectDidNotHappenSuccesses[clientIndex], - mqttDisconnectDidHappenSuccesses[clientIndex], - shutdownWasGracefulSuccesses[clientIndex], - mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex]), - clients.get(clientIndex)); - } + amqpDisconnectDidNotHappenSuccesses[clientIndex].setResult(true); //assume success until unexpected DISCONNECTED_RETRYING + mqttDisconnectDidHappenSuccesses[clientIndex].setResult(false); //assume failure until DISCONNECTED_RETRYING is triggered by token expiring + shutdownWasGracefulSuccesses[clientIndex].setResult(true); //assume success until DISCONNECTED callback without CLIENT_CLOSE + + mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex].setResult(false); //assume failure until first disconnected_retrying executes with reason EXPIRED_SAS_TOKEN + + clients.get(clientIndex).setConnectionStatusChangeCallback( + new IotHubConnectionStatusChangeTokenRenewalCallbackVerifier( + clients.get(clientIndex).getConfig().getProtocol(), + amqpDisconnectDidNotHappenSuccesses[clientIndex], + mqttDisconnectDidHappenSuccesses[clientIndex], + shutdownWasGracefulSuccesses[clientIndex], + mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex]), + clients.get(clientIndex)); + } - openEachClient(clients); - amqpMultiplexingClient.open(false); - amqpWsMultiplexingClient.open(false); + openEachClient(clients); + amqpMultiplexingClient.open(false); + amqpWsMultiplexingClient.open(false); - //wait until old sas token has expired, this should force the config to generate a new one from the device key - System.out.println("Sleeping..." + System.currentTimeMillis()); - Thread.sleep((SECONDS_FOR_SAS_TOKEN_TO_LIVE_BEFORE_RENEWAL + WAIT_BUFFER_FOR_TOKEN_TO_EXPIRE) * 1000); - System.out.println("Awake!" + System.currentTimeMillis()); + //wait until old sas token has expired, this should force the config to generate a new one from the device key + System.out.println("Sleeping..." + System.currentTimeMillis()); + Thread.sleep((SECONDS_FOR_SAS_TOKEN_TO_LIVE_BEFORE_RENEWAL + WAIT_BUFFER_FOR_TOKEN_TO_EXPIRE) * 1000); + System.out.println("Awake!" + System.currentTimeMillis()); - sendMessageFromEachClient(clients); + sendMessageFromEachClient(clients); - closeClients(clients); - amqpMultiplexingClient.close(); - amqpWsMultiplexingClient.close(); + closeClients(clients); + amqpMultiplexingClient.close(); + amqpWsMultiplexingClient.close(); - Tools.disposeTestIdentities(testIdentities, iotHubConnectionString); + Tools.disposeTestIdentities(testIdentities, iotHubConnectionString); - testIdentities.clear(); + testIdentities.clear(); + + verifyClientsConnectivityBehavedCorrectly(clients, amqpDisconnectDidNotHappenSuccesses, mqttDisconnectDidHappenSuccesses, shutdownWasGracefulSuccesses, mqttDisconnectHadTokenExpiredReasonSuccesses); + } + finally + { + // Without this, a failure part way through the test leaves behind every client that had already been + // opened. Those clients keep retrying their connections for the rest of the JVM's life, and the ones that + // were given proxy settings keep retrying through the proxy this class runs locally. That competes with + // the reruns of this test for the agent's resources and for that proxy, which makes the reruns fail the + // same way the first attempt did rather than giving them a clean chance to pass. + closeClientsQuietly(clients); + closeClientQuietly(amqpMultiplexingClient); + closeClientQuietly(amqpWsMultiplexingClient); + + // Identities are removed here as well so that a failed run does not leave them behind in the registry. + // This is a no-op when the test succeeded, because the successful path already disposed of them. + disposeTestIdentitiesQuietly(); + } + } - verifyClientsConnectivityBehavedCorrectly(clients, amqpDisconnectDidNotHappenSuccesses, mqttDisconnectDidHappenSuccesses, shutdownWasGracefulSuccesses, mqttDisconnectHadTokenExpiredReasonSuccesses); + /** + * Close each client, ignoring any failure. This is only used while cleaning up, where a client that cannot be + * closed must not replace the failure that the test is already reporting. + * + * @param clients The clients to close + */ + private void closeClientsQuietly(List clients) + { + for (InternalClient client : clients) + { + closeClientQuietly(client); + } + } + + private void closeClientQuietly(InternalClient client) + { + try + { + client.close(); + } + catch (UnsupportedOperationException ex) + { + // Multiplexed clients throw this when closed through the individual client itself. They are closed by + // closing the multiplexing client that they belong to instead. + } + catch (Exception ex) + { + log.debug("Failed to close a client while cleaning up after the test", ex); + } + } + + private void closeClientQuietly(MultiplexingClient multiplexingClient) + { + try + { + multiplexingClient.close(); + } + catch (Exception ex) + { + log.debug("Failed to close a multiplexing client while cleaning up after the test", ex); + } + } + + private void disposeTestIdentitiesQuietly() + { + try + { + Tools.disposeTestIdentities(testIdentities, iotHubConnectionString); + } + catch (Exception ex) + { + log.debug("Failed to dispose of the test identities while cleaning up after the test", ex); + } + + testIdentities.clear(); } private void closeClients(List clients) throws IOException From 99e506f6ca3e24cff7f0fdbfb1cdabeabac77d5e Mon Sep 17 00:00:00 2001 From: Ewerton Scaboro da Silva Date: Wed, 19 Aug 2026 11:33:39 -0700 Subject: [PATCH 2/2] Clean up when the test fails during setup as well The cleanup added in the previous commit only covered failures in the body of the test. Creating the clients registers identities as it goes, so a failure part way through creation leaked whatever had already been registered, and the instance field holding them was never cleared. RerunFailedTestRule re-evaluates the same test instance, so those entries were still present on the reruns. Moves the creation of the clients and of the two multiplexing clients inside the protected scope. The client references are declared beforehand, the multiplexing clients start as null, and the helper that closes them now returns early when it is given null, so a failure at any point during setup reaches the cleanup with whatever exists at that moment. createClientsToTest now adds to a list supplied by the caller rather than returning one at the end. Returning at the end meant that a failure during creation lost every client reference it had built up, leaving nothing to clean up. createMultiplexedClientToTest already worked this way, and the lists it fills are now declared alongside the other references so that partially created multiplexed clients are closed too, since they are only merged into the main list once both multiplexing clients exist. Also tracks the devices created for the custom sas token provider cases. Those are registered directly through the registry client rather than handed out by Tools, and they were never added to the identity list, so they were left behind in the registry on every run whether it passed or failed. They are ordinary SAS devices, so both disposal paths are correct for them: they are removed from the registry normally, and requeued for reuse when RECYCLE_TEST_IDENTITIES is set. Verified with mvn -pl iot-e2e-tests/common -am test-compile on JDK 8. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../sdk/iot/iothub/TokenRenewalTests.java | 52 +++++++++++++++---- 1 file changed, 42 insertions(+), 10 deletions(-) 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 a24bd38b58..e610ff07b9 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 @@ -113,15 +113,22 @@ public static void stopProxy() //@ContinuousIntegrationTest public void tokenRenewalWorks() throws Exception { - List clients = createClientsToTest(); - String hostname = clients.get(0).getConfig().getIotHubHostname(); + // These are declared before the try so that the cleanup below can reach whatever was created before a failure. + // Creating the clients registers identities as it goes, so a failure part way through creation has something + // to clean up just as much as a failure during the test body does. + List clients = new ArrayList<>(); ArrayList amqpMultiplexedClients = new ArrayList<>(); - MultiplexingClient amqpMultiplexingClient = createMultiplexedClientToTest(AMQPS, amqpMultiplexedClients, hostname); ArrayList amqpwsMultiplexedClients = new ArrayList<>(); - MultiplexingClient amqpWsMultiplexingClient = createMultiplexedClientToTest(AMQPS_WS, amqpwsMultiplexedClients, hostname); + MultiplexingClient amqpMultiplexingClient = null; + MultiplexingClient amqpWsMultiplexingClient = null; try { + createClientsToTest(clients); + String hostname = clients.get(0).getConfig().getIotHubHostname(); + amqpMultiplexingClient = createMultiplexedClientToTest(AMQPS, amqpMultiplexedClients, hostname); + amqpWsMultiplexingClient = createMultiplexedClientToTest(AMQPS_WS, amqpwsMultiplexedClients, hostname); + // Allow registry operations some buffer time before attempting to open connections for them Thread.sleep(2000); @@ -188,6 +195,13 @@ public void tokenRenewalWorks() throws Exception // the reruns of this test for the agent's resources and for that proxy, which makes the reruns fail the // same way the first attempt did rather than giving them a clean chance to pass. closeClientsQuietly(clients); + + // The multiplexed clients are cleaned up separately because they are only added to the list above once + // both multiplexing clients have been created. A failure before that point leaves them reachable only + // through these lists. + closeClientsQuietly(amqpMultiplexedClients); + closeClientsQuietly(amqpwsMultiplexedClients); + closeClientQuietly(amqpMultiplexingClient); closeClientQuietly(amqpWsMultiplexingClient); @@ -203,7 +217,7 @@ public void tokenRenewalWorks() throws Exception * * @param clients The clients to close */ - private void closeClientsQuietly(List clients) + private void closeClientsQuietly(List clients) { for (InternalClient client : clients) { @@ -230,6 +244,12 @@ private void closeClientQuietly(InternalClient client) private void closeClientQuietly(MultiplexingClient multiplexingClient) { + if (multiplexingClient == null) + { + // Creation did not get as far as this client + return; + } + try { multiplexingClient.close(); @@ -296,9 +316,17 @@ else if (protocol == AMQPS || protocol == AMQPS_WS) } } - private List createClientsToTest() throws IotHubException, IOException, URISyntaxException, InterruptedException, GeneralSecurityException + /** + * Create the clients that this test exercises, adding each one to the given list as it is created. + * + *

The clients are added to the caller's list rather than returned all at once so that a failure part way + * through creation still leaves the caller holding the clients that were created before it, which lets them be + * cleaned up rather than leaked.

+ * + * @param clients The list to add the created clients to + */ + private void createClientsToTest(List clients) throws IotHubException, IOException, URISyntaxException, InterruptedException, GeneralSecurityException { - List clients = new ArrayList<>(); Proxy testProxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress(testProxyHostname, testProxyPort)); for (IotHubClientProtocol protocol: IotHubClientProtocol.values()) { @@ -323,10 +351,14 @@ private List createClientsToTest() throws IotHubException, IOExc Device device = new Device(deviceId); device = registryClient.addDevice(device); SasTokenProvider sasTokenProvider = new SasTokenProviderImpl(Tools.getDeviceConnectionString(iotHubConnectionString, device), SECONDS_FOR_SAS_TOKEN_TO_LIVE_BEFORE_RENEWAL); - clients.add(new DeviceClient(iotHubHostName, deviceId, sasTokenProvider, protocol)); - } + DeviceClient clientWithCustomSasTokenProvider = new DeviceClient(iotHubHostName, deviceId, sasTokenProvider, protocol); - return clients; + // This device is registered here rather than handed out by Tools, so it has to be tracked alongside the + // other identities. Without this it is never disposed of and is left behind in the registry on every run, + // whether the test passes or fails. + testIdentities.add(new TestDeviceIdentity(clientWithCustomSasTokenProvider, device)); + clients.add(clientWithCustomSasTokenProvider); + } } private MultiplexingClient createMultiplexedClientToTest(IotHubClientProtocol protocol, List clientsToCreate, String hostname) throws IotHubException, IOException, URISyntaxException, InterruptedException, GeneralSecurityException, IotHubClientException, TimeoutException