Skip to content

fix: scheduling race with the QueryScheduler #1347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bdraco merged 8 commits into from
Dec 16, 2023
Merged

fix: scheduling race with the QueryScheduler #1347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c40a73b
fix: ensure correct scheduled query is removed from the heap with the…
bdraco Dec 16, 2023
f0fc8df
Merge branch 'master' into heappop_fix
bdraco Dec 16, 2023
54ce2d4
fix: ensure correct scheduled query is removed from the heap with the…
bdraco Dec 16, 2023
76b5aaf
Merge remote-tracking branch 'origin/heappop_fix' into heappop_fix
bdraco Dec 16, 2023
6bc9d05
fix: ensure correct scheduled query is removed from the heap with the…
bdraco Dec 16, 2023
94892fe
fix: always reschedule since records can get added twice due to race
bdraco Dec 16, 2023
3e8334d
fix: name
bdraco Dec 16, 2023
bd99f29
fix: remove now unreachable code
bdraco Dec 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/zeroconf/_services/browser.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ cdef class QueryScheduler:
cdef double _clock_resolution_millis
cdef object _question_type

cpdef void schedule_ptr_first_refresh(self, DNSPointer pointer)

cdef void _schedule_ptr_refresh(self, DNSPointer pointer, double expire_time_millis, double refresh_time_millis)

cdef void _schedule_ptr_query(self, _ScheduledPTRQuery scheduled_query)
Expand Down
13 changes: 3 additions & 10 deletions src/zeroconf/_services/browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,12 +374,6 @@ def stop(self) -> None:
self._next_scheduled_for_alias.clear()
self._query_heap.clear()

def schedule_ptr_first_refresh(self, pointer: DNSPointer) -> None:
"""Schedule a query for a pointer."""
expire_time_millis = pointer.get_expiration_time(100)
refresh_time_millis = pointer.get_expiration_time(_EXPIRE_REFRESH_TIME_PERCENT)
self._schedule_ptr_refresh(pointer, expire_time_millis, refresh_time_millis)

def _schedule_ptr_refresh(
self, pointer: DNSPointer, expire_time_millis: float_, refresh_time_millis: float_
) -> None:
Expand Down Expand Up @@ -415,6 +409,7 @@ def reschedule_ptr_first_refresh(self, pointer: DNSPointer) -> None:
):
return
current.cancelled = True
del self._next_scheduled_for_alias[pointer.alias]
expire_time_millis = pointer.get_expiration_time(100)
self._schedule_ptr_refresh(pointer, expire_time_millis, refresh_time_millis)

Expand Down Expand Up @@ -490,10 +485,8 @@ def _process_ready_types(self) -> None:
if query.when_millis > end_time_millis:
next_scheduled = query
break

query = heappop(self._query_heap)
ready_types.add(query.name)

heappop(self._query_heap)
del self._next_scheduled_for_alias[query.alias]
# If there is still more than 10% of the TTL remaining
# schedule a query again to try to rescue the record
Expand Down Expand Up @@ -670,7 +663,7 @@ def async_update_records(self, zc: 'Zeroconf', now: float_, records: List[Record
for type_ in self.types.intersection(cached_possible_types(pointer.name)):
if old_record is None:
self._enqueue_callback(SERVICE_STATE_CHANGE_ADDED, type_, pointer.alias)
self.query_scheduler.schedule_ptr_first_refresh(pointer)
self.query_scheduler.reschedule_ptr_first_refresh(pointer)
elif pointer.is_expired(now):
self._enqueue_callback(SERVICE_STATE_CHANGE_REMOVED, type_, pointer.alias)
self.query_scheduler.cancel_ptr_refresh(pointer)
Expand Down
6 changes: 3 additions & 3 deletions tests/services/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,15 +1185,15 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT, v6_flow_scope=()):
"disappear._hap._tcp.local.",
)

query_scheduler.schedule_ptr_first_refresh(ptr_record)
query_scheduler.reschedule_ptr_first_refresh(ptr_record)
expected_when_time = ptr_record.get_expiration_time(const._EXPIRE_REFRESH_TIME_PERCENT)
expected_expire_time = ptr_record.get_expiration_time(100)
ptr_query = _ScheduledPTRQuery(
ptr_record.alias, ptr_record.name, int(ptr_record.ttl), expected_expire_time, expected_when_time
)
assert query_scheduler._query_heap == [ptr_query]

query_scheduler.schedule_ptr_first_refresh(ptr2_record)
query_scheduler.reschedule_ptr_first_refresh(ptr2_record)
expected_when_time = ptr2_record.get_expiration_time(const._EXPIRE_REFRESH_TIME_PERCENT)
expected_expire_time = ptr2_record.get_expiration_time(100)
ptr2_query = _ScheduledPTRQuery(
Expand Down Expand Up @@ -1268,7 +1268,7 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT, v6_flow_scope=()):
"zoomer._hap._tcp.local.",
)

query_scheduler.schedule_ptr_first_refresh(ptr_record)
query_scheduler.reschedule_ptr_first_refresh(ptr_record)
expected_when_time = ptr_record.get_expiration_time(const._EXPIRE_REFRESH_TIME_PERCENT)
expected_expire_time = ptr_record.get_expiration_time(100)
ptr_query = _ScheduledPTRQuery(
Expand Down