diff --git a/CHANGES.md b/CHANGES.md index 2f8e24550..5fc06bed9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,6 @@ # HAP-Java 2.0.10 * Add optional Mute characteristic to Doorbell service +* Fix leak of mDNS instances and shutdown hooks when stopping `HomekitRoot` # HAP-Java 2.0.9 * Fix Set Duration and Remaining Duration characteristics to advertise format `uint32` instead of `int` [#202](https://github.com/hap-java/HAP-Java/issues/202) diff --git a/src/main/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiser.java b/src/main/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiser.java index 9086acb3e..1073f9b86 100644 --- a/src/main/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiser.java +++ b/src/main/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiser.java @@ -17,6 +17,8 @@ public class JmdnsHomekitAdvertiser { private static final String SERVICE_TYPE = "_hap._tcp.local."; private final JmDNS jmdns; + private final boolean ownsJmdns; + private Thread shutdownHook; private boolean discoverable = true; private static final Logger logger = LoggerFactory.getLogger(JmdnsHomekitAdvertiser.class); private boolean isAdvertising = false; @@ -30,11 +32,16 @@ public class JmdnsHomekitAdvertiser { private int category; public JmdnsHomekitAdvertiser(JmDNS jmdns) { - this.jmdns = jmdns; + this(jmdns, false); } public JmdnsHomekitAdvertiser(InetAddress localAddress) throws UnknownHostException, IOException { - jmdns = JmDNS.create(localAddress); + this(JmDNS.create(localAddress), true); + } + + JmdnsHomekitAdvertiser(JmDNS jmdns, boolean ownsJmdns) { + this.jmdns = jmdns; + this.ownsJmdns = ownsJmdns; } public synchronized void advertise( @@ -54,18 +61,33 @@ public synchronized void advertise( registerService(); - Runtime.getRuntime() - .addShutdownHook( - new Thread( - () -> { - logger.trace("Stopping advertising in response to shutdown."); - jmdns.unregisterAllServices(); - })); + shutdownHook = + new Thread( + () -> { + logger.trace("Stopping advertising in response to shutdown."); + jmdns.unregisterAllServices(); + }); + Runtime.getRuntime().addShutdownHook(shutdownHook); isAdvertising = true; } public synchronized void stop() { unregisterService(); + isAdvertising = false; + if (shutdownHook != null) { + try { + Runtime.getRuntime().removeShutdownHook(shutdownHook); + } catch (IllegalStateException e) { + // already shutting down + } + } + if (ownsJmdns) { + try { + jmdns.close(); + } catch (IOException e) { + logger.warn("Failed to close mDNS", e); + } + } } public synchronized void setDiscoverable(boolean discoverable) throws IOException { diff --git a/src/test/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiserTest.java b/src/test/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiserTest.java index 870bd739b..731be498f 100644 --- a/src/test/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiserTest.java +++ b/src/test/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiserTest.java @@ -2,7 +2,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import java.io.IOException; @@ -54,6 +56,29 @@ public void testSetConfigurationIndex() throws Exception { assertThat(getArgumentFromUnregister().getPropertyString("c#")).isEqualTo("1"); } + @Test + public void testStopClosesOwnedJmdns() throws Exception { + subject = new JmdnsHomekitAdvertiser(jmdns, true); + advertise(); + subject.stop(); + verify(jmdns).close(); + } + + @Test + public void testStopDoesNotCloseSuppliedJmdns() throws Exception { + advertise(); + subject.stop(); + verify(jmdns, never()).close(); + } + + @Test + public void testSetDiscoverableAfterStopDoesNotRegister() throws Exception { + advertise(); + subject.stop(); + subject.setDiscoverable(false); + verify(jmdns).registerService(any(ServiceInfo.class)); + } + private ServiceInfo getArgumentFromUnregister() { ArgumentCaptor serviceInfoCaptor = ArgumentCaptor.forClass(ServiceInfo.class); verify(jmdns).unregisterService(serviceInfoCaptor.capture());