Skip to content

fix: ha discovery workaround race - #14

Merged
jasaw merged 5 commits into
mainfrom
fix/ha_discovery_workaround_race
Oct 19, 2026
Merged

jasaw merged 5 commits into
mainfrom
fix/ha_discovery_workaround_race

Conversation

@jasaw

@jasaw jasaw commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

The workaround for HA zeroconf discovery introduced a rare race condition.

If a plug's PTR record is still cached but its SRV or A record has expired, we ask the plug for its records directly. If remove_service is called before the plug answers (e.g. because the PTR record expired), the removal is ignored,
since we don't know the plug's MAC yet, and the request keeps running. When the plug eventually answers, we add the device anyway, so it is reported as found after it was removed and is left stranded.

This PR fixes it by tracking each pending request by service name:

  • remove_service cancels a pending request, so the plug is never added.
  • Only one request per plug runs at a time. A repeated add_service or update_service while one is pending is ignored, as the pending request will dispatch the result.
  • Once the plug has answered, removal goes through the normal debounced path.

@jasaw
jasaw requested a review from jmattsson October 16, 2026 11:53
Comment thread src/powersensor_local/zeroconf_devices.py Outdated
self._owner._internal_callback(request())
lookup = self._owner._internal_callback(request())
self._lookups[name] = lookup
lookup.add_done_callback(lambda task: self._forget_lookup(name, task))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It smells weird to have lookup and task aliased here, but I don't see how to improve it.

Comment thread src/powersensor_local/zeroconf_devices.py Outdated
# The plug was never dispatched, so there is nothing to remove either.
on_add.assert_not_awaited()
on_remove.assert_not_awaited()
assert 'removal ignored' not in caplog.text

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there (also) be a positive assert for "removed before it answered"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the log, I mean

Comment thread tests/test_zeroconf_devices.py
@jasaw
jasaw merged commit c1ca68b into main Oct 19, 2026
1 check passed
@jasaw
jasaw deleted the fix/ha_discovery_workaround_race branch October 19, 2026 05:24
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.

2 participants