fix(sync): republish the mDNS record when the local address changes - #74
Merged
Conversation
Two defects, and the second is why the first was never noticed.
**readvertise() re-announced a snapshot.** `_info` is built once in `_advertise`
and the periodic refresh passed that same object to `update_service`, so after a
network change it rebroadcast the old address forever. It now compares the
current `_local_ip()` against the advertised one and rebuilds the record when
they differ. Loopback is excluded: `_local_ip()` returns 127.0.0.1 with no
route, and advertising that is worse than keeping a stale address.
**macOS never called it.** The GTK daemon has run this on a 180 s GLib timer;
`mac_app` and the headless `_run_mac` path had no equivalent, so a Mac
advertised whatever address it had at launch for its entire run. Both now
refresh on the same interval.
Measured: a laptop paired on the LAN joined an iPhone hotspot, registered
172.20.10.8, returned to the LAN and kept advertising the hotspot address. The
peer relearned it — that part is by design — and every send timed out:
text: send to <peer> FAILED after 3x over [('172.20.10.8', 47823)]: TimeoutError
One candidate in that list, not two, because the advertisement had also
overwritten the stored fallback. Only restarting the app cleared it.
ServiceInfo construction moves into `_service_info()` so the refresh can't drift
from the original registration — the same duplication that let the two staging
directories collapse into one.
scripts/sync_readvertise_test.py (new, in CI) drives readvertise against a fake
zeroconf: a changed address is republished, an unchanged one re-announces
without churning the record, loopback is never advertised, the
update_service-fails path still gets the new record out (and unregisters the
old), and TXT properties survive the rebuild. It fails on the old code with
"still advertising 172.20.10.8".
Not addressed: `_on_zc` still overwrites the stored peer address with whatever
was advertised, so a bad advertisement still collapses `_peer_addrs`' two
candidates into one. That's the other half, and it wants a known-good address
kept separately.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two defects, and the second is why the first went unnoticed.
1.
readvertise()re-announced a snapshot_infois built once in_advertise(), and the periodic refresh handed that same object back toupdate_service— so after a network change it rebroadcast the old address indefinitely.It now compares the current
_local_ip()against the advertised one and rebuilds when they differ. Loopback is excluded:_local_ip()returns127.0.0.1with no route, and advertising that is worse than keeping a stale address.2. macOS never called it at all
The GTK daemon has run this on a 180 s
GLibtimer.mac_appand the headless_run_macpath had no equivalent, so a Mac advertised whatever address it had at launch for its entire run — which is exactly where this bit. Both now refresh on the same interval.What it looked like
A laptop paired on the LAN joined an iPhone hotspot, registered
172.20.10.8, came back to the LAN, and kept advertising the hotspot address. The peer relearned it — that part is by design — and every send timed out:Note the single candidate: the advertisement had also overwritten the stored fallback. Only restarting the app cleared it.
ServiceInfoconstruction moves into_service_info()so the refresh can't drift from the original registration — the same duplication that let the two staging directories collapse into one in #72.Tests
scripts/sync_readvertise_test.py(new, in CI) drivesreadvertiseagainst a fake zeroconf:_local_ip()→ loopbackupdate_serviceraisesid/name/fpsurvive the rebuildAgainst the old code it fails with
still advertising 172.20.10.8 — this is the bug. Full suite green locally (7 scripts).Not addressed
_on_zcstill overwrites the stored peer address with whatever was advertised, so a bad advertisement collapses_peer_addrs' two candidates into one — which is why the log above shows one address instead of two. That's the other half of the resilience story and wants a known-good address kept separately; worth its own change.Verifying the real-world path needs an actual network change on a Mac — the test covers the decision logic, not the wire.