diff --git a/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2MTransportBootstrapService.java b/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2MTransportBootstrapService.java index 6de8acde90..a9a4e364b5 100644 --- a/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2MTransportBootstrapService.java +++ b/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2MTransportBootstrapService.java @@ -184,21 +184,33 @@ public class LwM2MTransportBootstrapService implements SmartInitializingSingleto log.info("Creating new LwM2M Bootstrap server with updated certificates..."); LeshanBootstrapServer newServer = getLhBootstrapServer(); + + // Stop the old server first to release the ports before starting the new one + if (oldServer != null) { + log.info("Stopping old LwM2M Bootstrap server to release ports..."); + oldServer.destroy(); + } + try { newServer.start(); } catch (Exception e) { - log.error("Failed to start new LwM2M Bootstrap server, rolling back", e); + log.error("Failed to start new LwM2M Bootstrap server", e); newServer.destroy(); + // Attempt to restore the old server + if (oldServer != null) { + try { + LeshanBootstrapServer restoredServer = getLhBootstrapServer(); + restoredServer.start(); + this.server = restoredServer; + log.info("Restored LwM2M Bootstrap server with previous configuration."); + } catch (Exception restoreEx) { + log.error("Failed to restore old LwM2M Bootstrap server", restoreEx); + } + } throw e; } this.server = newServer; log.info("New LwM2M Bootstrap server started successfully."); - - if (oldServer != null) { - log.info("Stopping old LwM2M Bootstrap server..."); - oldServer.destroy(); - log.info("Old LwM2M Bootstrap server stopped."); - } } } diff --git a/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/server/DefaultLwM2mTransportService.java b/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/server/DefaultLwM2mTransportService.java index 91b3fb072f..b0d25729b3 100644 --- a/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/server/DefaultLwM2mTransportService.java +++ b/common/transport/lwm2m/src/main/java/org/thingsboard/server/transport/lwm2m/server/DefaultLwM2mTransportService.java @@ -235,33 +235,10 @@ public class DefaultLwM2mTransportService implements LwM2MTransportService, Smar log.info("Creating new LwM2M server with updated certificates..."); LeshanServer newServer = getLhServer(); - try { - newServer.start(); - } catch (Exception e) { - log.error("Failed to start new LwM2M server, rolling back", e); - newServer.destroy(); - throw e; - } - - try { - LwM2mServerListener newListener = new LwM2mServerListener(handler); - newServer.getRegistrationService().addListener(newListener.registrationListener); - newServer.getPresenceService().addListener(newListener.presenceListener); - newServer.getObservationService().addListener(newListener.observationListener); - newServer.getSendService().addListener(newListener.sendListener); - - this.server = newServer; - this.context.setServer(newServer); - this.serverListener = newListener; - } catch (Exception e) { - log.error("Failed to register listeners on new LwM2M server, rolling back", e); - newServer.destroy(); - throw e; - } - log.info("New LwM2M server started successfully."); + // Stop old server first to release the ports before starting the new one if (oldServer != null) { - log.info("Stopping old LwM2M server..."); + log.info("Stopping old LwM2M server to release ports..."); if (oldListener != null) { oldServer.getRegistrationService().removeListener(oldListener.registrationListener); oldServer.getPresenceService().removeListener(oldListener.presenceListener); @@ -269,8 +246,42 @@ public class DefaultLwM2mTransportService implements LwM2MTransportService, Smar oldServer.getSendService().removeListener(oldListener.sendListener); } oldServer.destroy(); - log.info("Old LwM2M server stopped."); } + + try { + newServer.start(); + } catch (Exception e) { + log.error("Failed to start new LwM2M server", e); + newServer.destroy(); + // Attempt to restore the old server + try { + LeshanServer restoredServer = getLhServer(); + restoredServer.start(); + LwM2mServerListener restoredListener = new LwM2mServerListener(handler); + restoredServer.getRegistrationService().addListener(restoredListener.registrationListener); + restoredServer.getPresenceService().addListener(restoredListener.presenceListener); + restoredServer.getObservationService().addListener(restoredListener.observationListener); + restoredServer.getSendService().addListener(restoredListener.sendListener); + this.server = restoredServer; + this.context.setServer(restoredServer); + this.serverListener = restoredListener; + log.info("Restored LwM2M server with previous configuration."); + } catch (Exception restoreEx) { + log.error("Failed to restore old LwM2M server", restoreEx); + } + throw e; + } + + LwM2mServerListener newListener = new LwM2mServerListener(handler); + newServer.getRegistrationService().addListener(newListener.registrationListener); + newServer.getPresenceService().addListener(newListener.presenceListener); + newServer.getObservationService().addListener(newListener.observationListener); + newServer.getSendService().addListener(newListener.sendListener); + + this.server = newServer; + this.context.setServer(newServer); + this.serverListener = newListener; + log.info("New LwM2M server started successfully."); } @Override diff --git a/common/transport/lwm2m/src/test/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2mBootstrapCertificateReloadTest.java b/common/transport/lwm2m/src/test/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2mBootstrapCertificateReloadTest.java index 51d37ca739..84bc44b03b 100644 --- a/common/transport/lwm2m/src/test/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2mBootstrapCertificateReloadTest.java +++ b/common/transport/lwm2m/src/test/java/org/thingsboard/server/transport/lwm2m/bootstrap/LwM2mBootstrapCertificateReloadTest.java @@ -118,8 +118,8 @@ public class LwM2mBootstrapCertificateReloadTest { Runnable reloadCallback = callbackCaptor.getValue(); - // getLhBootstrapServer() will fail due to null host. - // With create-then-swap, the old server should NOT be destroyed. + // getLhBootstrapServer() will fail due to null host before old server is stopped. + // The old server should NOT be destroyed since the new server was never created. reloadCallback.run(); verify(mockBootstrapServer, never()).destroy(); @@ -164,15 +164,18 @@ public class LwM2mBootstrapCertificateReloadTest { } @Test - public void givenReloadCallback_whenNewServerStartFails_thenNewServerDestroyedAndOldPreserved() { + public void givenReloadCallback_whenNewServerStartFails_thenNewServerDestroyedAndRestorationAttempted() { // GIVEN ReflectionTestUtils.setField(bootstrapService, "server", mockBootstrapServer); LeshanBootstrapServer mockNewServer = mock(LeshanBootstrapServer.class); doThrow(new RuntimeException("start failed")).when(mockNewServer).start(); + LeshanBootstrapServer mockRestoredServer = mock(LeshanBootstrapServer.class); + LwM2MTransportBootstrapService spyService = Mockito.spy(bootstrapService); - doReturn(mockNewServer).when(spyService).getLhBootstrapServer(); + // First call returns the failing server, second call returns the restoration server + doReturn(mockNewServer).doReturn(mockRestoredServer).when(spyService).getLhBootstrapServer(); ArgumentCaptor callbackCaptor = ArgumentCaptor.forClass(Runnable.class); spyService.afterSingletonsInstantiated(); @@ -184,9 +187,13 @@ public class LwM2mBootstrapCertificateReloadTest { reloadCallback.run(); // THEN + // Old server is destroyed to release ports + verify(mockBootstrapServer).destroy(); + // New server fails to start and is destroyed verify(mockNewServer).destroy(); - assertThat(ReflectionTestUtils.getField(spyService, "server")).isSameAs(mockBootstrapServer); - verify(mockBootstrapServer, never()).destroy(); + // Restoration server is started and becomes the active server + verify(mockRestoredServer).start(); + assertThat(ReflectionTestUtils.getField(spyService, "server")).isSameAs(mockRestoredServer); } } diff --git a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/CertificateReloadManagerTest.java b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/CertificateReloadManagerTest.java index 0c9f1fc5b1..ba3aa66c70 100644 --- a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/CertificateReloadManagerTest.java +++ b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/CertificateReloadManagerTest.java @@ -63,7 +63,7 @@ public class CertificateReloadManagerTest { reloadLatch.countDown(); }); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nTEST_CERT_V2_MODIFIED\n-----END CERTIFICATE-----\n"); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -80,9 +80,9 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); assertThat(reloadCount.get()).isEqualTo(0); @@ -94,7 +94,7 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -107,13 +107,13 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.delete(certFile); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); // File deletion changes checksum from real hash to "", so reload is triggered assertThat(reloadCount.get()).isEqualTo(1); @@ -139,9 +139,9 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert", certFile, certReloadLatch::countDown); certificateReloadManager.registerWatcher("test-key", keyFile, keyReloadLatch::countDown); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(keyFile, "-----BEGIN PRIVATE KEY-----\nTEST_KEY_V2_MODIFIED\n-----END PRIVATE KEY-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -162,10 +162,10 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert1", certFile, reload1Count::incrementAndGet); certificateReloadManager.registerWatcher("test-cert2", cert2File, reload2Count::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n"); Files.writeString(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -186,10 +186,10 @@ public class CertificateReloadManagerTest { }); certificateReloadManager.registerWatcher("test-cert2", cert2File, reload2Count::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED1\n-----END CERTIFICATE-----\n"); Files.writeString(cert2File, "-----BEGIN CERTIFICATE-----\nMODIFIED2\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -203,15 +203,15 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.delete(certFile); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nNEW_CERT\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -229,12 +229,12 @@ public class CertificateReloadManagerTest { reloadLatch.countDown(); }); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); for (int i = 0; i < 5; i++) { Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nCERT_VERSION_" + i + "\n-----END CERTIFICATE-----\n"); } - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -252,9 +252,9 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nMODIFIED\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); for (int i = 0; i < 5; i++) { new Thread(() -> { @@ -284,10 +284,10 @@ public class CertificateReloadManagerTest { certificateReloadManager.registerWatcher("test-cert", certFile, reloadCount::incrementAndGet); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, originalContent); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -304,9 +304,9 @@ public class CertificateReloadManagerTest { throw new RuntimeException("Persistent failure"); }); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); // Retry up to MAX_CONSECUTIVE_FAILURES (10) + a few extra to confirm it stops for (int i = 0; i < 15; i++) { @@ -328,9 +328,9 @@ public class CertificateReloadManagerTest { } }); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); // First attempt fails ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -338,9 +338,9 @@ public class CertificateReloadManagerTest { // Fix the callback and change the file to new content shouldFail.set(0); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nGOOD_CERT\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); // Should reset failure counter and succeed because file content changed ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates"); @@ -359,9 +359,9 @@ public class CertificateReloadManagerTest { } }); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nBAD_CERT\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); // Exhaust all retries for (int i = 0; i < 15; i++) { @@ -371,9 +371,9 @@ public class CertificateReloadManagerTest { // Fix callback and change file to new content shouldFail.set(0); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); Files.writeString(certFile, "-----BEGIN CERTIFICATE-----\nFIXED_CERT\n-----END CERTIFICATE-----\n"); - Thread.sleep(100); + TimeUnit.MILLISECONDS.sleep(100); // Should detect new content, reset counter, and succeed ReflectionTestUtils.invokeMethod(certificateReloadManager, "checkCertificates");