Skip to content
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.

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

Move thread safety check in async_register/async_remove #116077

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 40 additions & 4 deletions homeassistant/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2456,7 +2456,7 @@ def register(
"""
run_callback_threadsafe(
self._hass.loop,
self.async_register,
self._async_register,
domain,
service,
service_func,
Expand Down Expand Up @@ -2484,6 +2484,33 @@ def async_register(

Schema is called to coerce and validate the service data.

This method must be run in the event loop.
"""
self._hass.verify_event_loop_thread("async_register")
self._async_register(
domain, service, service_func, schema, supports_response, job_type
)

@callback
def _async_register(
self,
domain: str,
service: str,
service_func: Callable[
[ServiceCall],
Coroutine[Any, Any, ServiceResponse | EntityServiceResponse]
| ServiceResponse
| EntityServiceResponse
| None,
],
schema: vol.Schema | None = None,
supports_response: SupportsResponse = SupportsResponse.NONE,
job_type: HassJobType | None = None,
) -> None:
"""Register a service.

Schema is called to coerce and validate the service data.

This method must be run in the event loop.
"""
domain = domain.lower()
Expand All @@ -2502,20 +2529,29 @@ def async_register(
else:
self._services[domain] = {service: service_obj}

self._hass.bus.async_fire(
self._hass.bus.async_fire_internal(
EVENT_SERVICE_REGISTERED, {ATTR_DOMAIN: domain, ATTR_SERVICE: service}
)

def remove(self, domain: str, service: str) -> None:
"""Remove a registered service from service handler."""
run_callback_threadsafe(
self._hass.loop, self.async_remove, domain, service
self._hass.loop, self._async_remove, domain, service
).result()

@callback
def async_remove(self, domain: str, service: str) -> None:
"""Remove a registered service from service handler.

This method must be run in the event loop.
"""
self._hass.verify_event_loop_thread("async_remove")
self._async_remove(domain, service)

@callback
def _async_remove(self, domain: str, service: str) -> None:
"""Remove a registered service from service handler.

This method must be run in the event loop.
"""
domain = domain.lower()
Expand All @@ -2530,7 +2566,7 @@ def async_remove(self, domain: str, service: str) -> None:
if not self._services[domain]:
self._services.pop(domain)

self._hass.bus.async_fire(
self._hass.bus.async_fire_internal(
EVENT_SERVICE_REMOVED, {ATTR_DOMAIN: domain, ATTR_SERVICE: service}
)

Expand Down
23 changes: 23 additions & 0 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -3457,3 +3457,26 @@ async def test_async_fire_thread_safety(hass: HomeAssistant) -> None:
await hass.async_add_executor_job(hass.bus.async_fire, "test_event")

assert len(events) == 1


async def test_async_register_thread_safety(hass: HomeAssistant) -> None:
"""Test async_register thread safety."""
with pytest.raises(
RuntimeError, match="Detected code that calls async_register from a thread."
):
await hass.async_add_executor_job(
hass.services.async_register,
"test_domain",
"test_service",
lambda call: None,
)


async def test_async_remove_thread_safety(hass: HomeAssistant) -> None:
"""Test async_remove thread safety."""
with pytest.raises(
RuntimeError, match="Detected code that calls async_remove from a thread."
):
await hass.async_add_executor_job(
hass.services.async_remove, "test_domain", "test_service"
)