Skip to content

Close owned mDNS instance and remove shutdown hook on stop - #214

Open
yfre wants to merge 1 commit into
masterfrom
fix/close-owned-jmdns
Open

yfre wants to merge 1 commit into
masterfrom
fix/close-owned-jmdns

Conversation

@yfre

@yfre yfre commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Problem

When HomekitServer is created with an InetAddress, JmdnsHomekitAdvertiser creates its own JmDNS instance. HomekitRoot.stop() never closed it, and every advertise() registered a shutdown hook that was never removed. Each stopped root therefore left a running JmDNS instance (with its threads) behind, kept reachable by the shutdown hook. Applications that stop and recreate the bridge, e.g. on network changes, accumulate these instances over time.

Change

JmdnsHomekitAdvertiser.stop() now:

  • removes the shutdown hook registered by advertise()
  • closes the JmDNS instance only if the advertiser created it; a JmDNS passed in by the caller stays open
  • resets the advertising state, so setDiscoverable, setMac and setConfigurationIndex after stop() do not re-register the service

No public API changes.

Tests

  • testStopClosesOwnedJmdns
  • testStopDoesNotCloseSuppliedJmdns
  • testSetDiscoverableAfterStopDoesNotRegister

The first and third fail without the fix.

JmdnsHomekitAdvertiser created its own JmDNS when given an InetAddress but
never closed it, and registered a shutdown hook on every advertise() that was
never removed. Each stopped HomekitRoot therefore left a running JmDNS
instance behind, kept reachable by its shutdown hook.

stop() now removes the shutdown hook, closes the JmDNS only if the advertiser
created it, and resets the advertising state so later changes to
discoverability, MAC or configuration index do not re-register the service.
@yfre
yfre requested a review from ccutrer November 7, 2026 12:36

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant