Skip to content

Commit 28938d2

Browse files
authored
Throw NotRunningException when Zeroconf is not running (#1033)
- Before this change the consumer would get a timeout or an EventLoopBlocked exception when calling `ServiceInfo.*request` when the instance had already been shutdown. This was quite a confusing result.
1 parent 21bd107 commit 28938d2

6 files changed

Lines changed: 41 additions & 14 deletions

File tree

tests/test_asyncio.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
DNSService,
2424
DNSAddress,
2525
DNSText,
26+
NotRunningException,
2627
ServiceStateChange,
2728
Zeroconf,
2829
const,
@@ -1090,6 +1091,15 @@ async def test_async_request_timeout():
10901091
assert (end_time - start_time) < 3000 + 1000
10911092

10921093

1094+
@pytest.mark.asyncio
1095+
async def test_async_request_non_running_instance():
1096+
"""Test that the async_request throws when zeroconf is not running."""
1097+
aiozc = AsyncZeroconf(interfaces=['127.0.0.1'])
1098+
await aiozc.async_close()
1099+
with pytest.raises(NotRunningException):
1100+
await aiozc.async_get_service_info("_notfound.local.", "notthere._notfound.local.")
1101+
1102+
10931103
@pytest.mark.asyncio
10941104
async def test_legacy_unicast_response(run_isolated):
10951105
"""Verify legacy unicast responses include questions and correct id."""

zeroconf/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
IncomingDecodeError,
4545
NamePartTooLongException,
4646
NonUniqueNameException,
47+
NotRunningException,
4748
ServiceNameAlreadyRegistered,
4849
)
4950
from ._logger import QuietLogger, log # noqa # import needed for backwards compat
@@ -101,6 +102,7 @@
101102
"IncomingDecodeError",
102103
"NamePartTooLongException",
103104
"NonUniqueNameException",
105+
"NotRunningException",
104106
"ServiceNameAlreadyRegistered",
105107
]
106108

zeroconf/_core.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232

3333
from ._cache import DNSCache
3434
from ._dns import DNSQuestion, DNSQuestionType
35-
from ._exceptions import NonUniqueNameException
35+
from ._exceptions import NonUniqueNameException, NotRunningException
3636
from ._handlers import (
3737
MulticastOutgoingQueue,
3838
QueryHandler,
@@ -80,6 +80,7 @@
8080
_MDNS_PORT,
8181
_ONE_SECOND,
8282
_REGISTER_TIME,
83+
_STARTUP_TIMEOUT,
8384
_TYPE_PTR,
8485
_UNREGISTER_TIME,
8586
)
@@ -118,15 +119,15 @@ def __init__(
118119
self.protocols: List[AsyncListener] = []
119120
self.readers: List[asyncio.DatagramTransport] = []
120121
self.senders: List[asyncio.DatagramTransport] = []
122+
self.running_event: Optional[asyncio.Event] = None
121123
self._listen_socket = listen_socket
122124
self._respond_sockets = respond_sockets
123125
self._cleanup_timer: Optional[asyncio.TimerHandle] = None
124-
self._running_event: Optional[asyncio.Event] = None
125126

126127
def setup(self, loop: asyncio.AbstractEventLoop, loop_thread_ready: Optional[threading.Event]) -> None:
127128
"""Set up the instance."""
128129
self.loop = loop
129-
self._running_event = asyncio.Event()
130+
self.running_event = asyncio.Event()
130131
self.loop.create_task(self._async_setup(loop_thread_ready))
131132

132133
async def _async_setup(self, loop_thread_ready: Optional[threading.Event]) -> None:
@@ -136,16 +137,11 @@ async def _async_setup(self, loop_thread_ready: Optional[threading.Event]) -> No
136137
millis_to_seconds(_CACHE_CLEANUP_INTERVAL), self._async_cache_cleanup
137138
)
138139
await self._async_create_endpoints()
139-
assert self._running_event is not None
140-
self._running_event.set()
140+
assert self.running_event is not None
141+
self.running_event.set()
141142
if loop_thread_ready:
142143
loop_thread_ready.set()
143144

144-
async def async_wait_for_start(self) -> None:
145-
"""Wait for start up."""
146-
assert self._running_event is not None
147-
await self._running_event.wait()
148-
149145
async def _async_create_endpoints(self) -> None:
150146
"""Create endpoints to send and receive."""
151147
assert self.loop is not None
@@ -495,8 +491,17 @@ def _run_loop() -> None:
495491
loop_thread_ready.wait()
496492

497493
async def async_wait_for_start(self) -> None:
498-
"""Wait for start up."""
499-
await self.engine.async_wait_for_start()
494+
"""Wait for start up for actions that require a running Zeroconf instance.
495+
496+
Throws NotRunningException if the instance is not running or could
497+
not be started.
498+
"""
499+
if self.done: # If the instance was shutdown from under us, raise immediately
500+
raise NotRunningException
501+
assert self.engine.running_event is not None
502+
await wait_event_or_timeout(self.engine.running_event, timeout=_STARTUP_TIMEOUT)
503+
if not self.engine.running_event.is_set() or self.done:
504+
raise NotRunningException
500505

501506
@property
502507
def listeners(self) -> List[RecordUpdateListener]:

zeroconf/_exceptions.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,11 @@ class EventLoopBlocked(Error):
5757
when the cpu is maxed out or there is something blocking
5858
the event loop.
5959
"""
60+
61+
62+
class NotRunningException(Error):
63+
"""Exception when an action is called with a zeroconf instance that is not running.
64+
65+
The instance may not be running because it was already shutdown
66+
or startup has failed in some unexpected way.
67+
"""

zeroconf/asyncio.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,9 @@ async def async_update_service(self, info: ServiceInfo) -> Awaitable:
218218
async def async_close(self) -> None:
219219
"""Ends the background threads, and prevent this instance from
220220
servicing further queries."""
221-
with contextlib.suppress(asyncio.TimeoutError):
222-
await asyncio.wait_for(self.zeroconf.async_wait_for_start(), timeout=1)
221+
if not self.zeroconf.done:
222+
with contextlib.suppress(asyncio.TimeoutError):
223+
await asyncio.wait_for(self.zeroconf.async_wait_for_start(), timeout=1)
223224
await self.async_remove_all_service_listeners()
224225
await self.async_unregister_all_services()
225226
await self.zeroconf._async_close() # pylint: disable=protected-access

zeroconf/const.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
_BROWSER_BACKOFF_LIMIT = 3600 # s
3535
_CACHE_CLEANUP_INTERVAL = 10000 # ms
3636
_LOADED_SYSTEM_TIMEOUT = 10 # s
37+
_STARTUP_TIMEOUT = 9 # s must be lower than _LOADED_SYSTEM_TIMEOUT
3738
_ONE_SECOND = 1000 # ms
3839

3940
# If the system is loaded or the event

0 commit comments

Comments
 (0)