search

thlucas1 / homeassistantcomponent_spotifyplus

Home Assistant integration for Spotify Player control, services, and soundtouchplus integration support.
MIT License
45 stars 3 forks source link

HA 2026.6 - async_update_entry from a thread other than the event loop #15

Closed Thrasher2020 closed 3 months ago

Thrasher2020 commented 4 months ago

System Health details

System Information

version core-2024.6.0b1
installation_type Home Assistant OS
dev false
hassio true
docker true
user root
virtualenv false
python_version 3.12.2
os_name Linux
os_version 6.6.28-haos-raspi
arch aarch64
timezone Europe/London
config_dir /config
Home Assistant Community Store GitHub API | ok -- | -- GitHub Content | ok GitHub Web | ok GitHub API Calls Remaining | 5000 Installed Version | 1.34.0 Stage | running Available Repositories | 1478 Downloaded Repositories | 72 HACS Data | ok
Home Assistant Cloud logged_in | true -- | -- subscription_expiration | 17 June 2024 at 01:00 relayer_connected | true relayer_region | eu-central-1 remote_enabled | true remote_connected | true alexa_enabled | true google_enabled | true remote_server | eu-central-1-10.ui.nabu.casa certificate_status | ready instance_id | 7a6e260cca76465b8d6c48b7497998f9 can_reach_cert_server | ok can_reach_cloud_auth | ok can_reach_cloud | ok
Home Assistant Supervisor host_os | Home Assistant OS 12.3 -- | -- update_channel | beta supervisor_version | supervisor-2024.05.2 agent_version | 1.6.0 docker_version | 25.0.5 disk_total | 228.5 GB disk_used | 36.1 GB healthy | true supported | true host_connectivity | true supervisor_connectivity | true ntp_synchronized | true virtualization | board | rpi4-64 supervisor_api | ok version_api | ok installed_addons | Samba share (12.3.1), Grafana (10.0.0), File editor (5.8.0), TasmoAdmin (0.30.2), InfluxDB (5.0.0), Advanced SSH & Web Terminal (18.0.0), Home Assistant Google Drive Backup (0.112.1), Zigbee2MQTT (1.37.1-1), chrony (3.0.1), UniFi Network Application (3.0.5), Nginx Proxy Manager (1.0.1), Tailscale (0.19.1), PS5 MQTT (1.3.3), ESPHome (2024.5.4), Mosquitto broker (6.4.1), Glances (0.21.1), AppDaemon (0.16.6), Increase Swap (1.1.3), SQLite Web (4.1.2), MQTT Explorer (browser-1.0.3)
Dashboards dashboards | 9 -- | -- resources | 34 views | 22 mode | storage
Recorder oldest_recorder_run | 27 May 2024 at 09:06 -- | -- current_recorder_run | 30 May 2024 at 08:27 estimated_db_size | 2213.37 MiB database_engine | sqlite database_version | 3.44.2
Solcast PV Forecast can_reach_server | ok -- | -- used_requests | 3 rooftop_site_count | 1
Sonoff version | 3.7.3 (e240aaf) -- | -- cloud_online | 0 / 2 local_online | 1 / 1
SpotifyPlus integration_version | v1.0.19 -- | -- clients_configured | 1: Neil Brownlee (premium) api_endpoint_reachable | ok

Checklist

Describe the issue

HA2024.6.0b1 - errors reported when trying to use the integration. Log below.

Reproduction steps

  1. Automation fails.

  3. ...

Debug logs

Logger: homeassistant.components.automation.song_to_spotify_office
Source: components/automation/__init__.py:744
integration: Automation (documentation, issues)
First occurred: 09:33:16 (10 occurrences)
Last logged: 10:33:48

Error while executing automation automation.song_to_spotify_office: SAM0001E - An unhandled exception occured while processing method "MakeRequest". Detected that custom integration 'spotifyplus' calls hass.config_entries.async_update_entry from a thread other than the event loop, which may cause Home Assistant to crash or data to corrupt. For more information, see https://developers.home-assistant.io/docs/asyncio_thread_safety/#hassconfig_entriesasync_update_entry at custom_components/spotifyplus/__init__.py, line 1766: session.hass.config_entries.async_update_entry(. Please report it to the author of the 'spotifyplus' custom integration.

Diagnostics dump

No response

thlucas1 commented 4 months ago

Thanks for bringing this to my attention. The fix will have to wait until Mid next week, as I am currently traveling.

It appears that the HA core team pulled another fast one on me with the 06 release like they did with the 05 release. This integration has been working fine since it was released, and now they are starting to enforce checks for updating state in the event loop! Should be an easy fix. Will keep you posted..

Thrasher2020 commented 4 months ago

Thanks - enjoy your holiday :)

thlucas1 commented 3 months ago

@bdraco I'm running into an issue with the HA 2024.6 release, specifically trying to call async_update_entry from a thread not on the event loop. I now understand that updating a config entry must be done in the event loop thread, as there is no sync API to update config entries. I am now trying to use hass.add_job to schedule a function in the event loop that calls hass.config_entries.async_update_entry.

The problem I am having is passing a keyword argument to the async_update_entry method - old call looks like this:

session.hass.config_entries.async_update_entry(
    session.config_entry, 
    data={**session.config_entry.data, "token": token}
    )

I am trying to replace it with the following:

session.hass.add_job(
    session.hass.config_entries.async_update_entry,
    session.config_entry, 
    data={**session.config_entry.data, "token": token}
)

The replaced code is generating the following exception (logged in the System log) when trying to execute:

Error doing job: Exception in callback ConfigEntries.async_update_entry(<ConfigEntry ...xfrmd4bpfhqke>, {'auth_implementation': 'spotifyplus_...6ecc68097a218', 'description': '(Premium account)', 'id': 'xxxxxx', 'name': 'Todd L', ...})
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/asyncio/events.py", line 88, in _run
    self._context.run(self._callback, *self._args)
TypeError: ConfigEntries.async_update_entry() takes 2 positional arguments but 3 were given

Is there a way to pass keyword arguments to the hass.add_job method?

thlucas1 commented 3 months ago

Renamed the thread to better reflect the underlying error. I am back from vacation, and working on a fix for this issue.

bdraco commented 3 months ago

You can use functools.partial

thlucas1 commented 3 months ago

You can use functools.partial

@bdraco That seemed to do the trick - thank you so much!

bdraco commented 3 months ago

Make sure the job you are running is wrapped in @callback or it will run in the executor

thlucas1 commented 3 months ago

Make sure the job you are running is wrapped in @callback or it will run in the executor @bdraco It is calling the async_update_entry, which is marked with a @callback:


@callback
def async_update_entry(
self,
entry: ConfigEntry,
*,
data: Mapping[str, Any] | UndefinedType = UNDEFINED,
minor_version: int | UndefinedType = UNDEFINED,
options: Mapping[str, Any] | UndefinedType = UNDEFINED,
pref_disable_new_entities: bool | UndefinedType = UNDEFINED,
pref_disable_polling: bool | UndefinedType = UNDEFINED,
title: str | UndefinedType = UNDEFINED,
unique_id: str | None | UndefinedType = UNDEFINED,
version: int | UndefinedType = UNDEFINED,
) -> bool:
if entry.entry_id not in self._entries:
raise UnknownEntry(entry.entry_id)
    self.hass.verify_event_loop_thread("hass.config_entries.async_update_entry")


Thanks again for your assistance.
thlucas1 commented 3 months ago

@Thrasher2020 Fixed with release v1.0.20 of the SpotifyPlus integration

Give it a try and let me know if you have issues. Thanks.

Thrasher2020 commented 3 months ago

Testing right now. Will let you know :)

Thank you.

thlucas1 commented 3 months ago

Make sure the job you are running is wrapped in @callback or it will run in the executor

@bdraco Just curious - are there performance (or multi-threading) consequences to running a job in the executor, versus the @callback method? Any HA documentation that I could look at to learn more about the differences or when to use one over the other?

It seems to be running fine with the @callback method, but I have other methods that I am calling that all use the await hass.async_add_executor_job(...) syntax. I think I have to use that syntax in these cases, as they are calls to underlying API's that do not support async, but not sure.

I appreciate your help and guidance.

bdraco commented 3 months ago

hass.add_job is generally the way to get back to the async (main thread). - as long as its decorated with @callback https://github.com/home-assistant/core/blob/8c025ea1f7f254251d5d63f436b6da27363c83e4/homeassistant/core.py#L580 https://github.com/home-assistant/core/blob/8c025ea1f7f254251d5d63f436b6da27363c83e4/homeassistant/core.py#L745

await hass.async_add_executor_job is generally the way to offload work that would block the event loop to another thread. There is a limit to 64 executor jobs running at the same time. It also needs to create a future, and schedule it on the other thread, wait for the result, and safely return it to the main thread so there is some additional cost for doing so which sometimes is even more expensive than the work being done.

Ideally everything is async and runs in the main thread and you don't have the overhead of switching between threads since the cost of doing it safely can comparatively be rather high for short/non-cpu bound operations. Also if everything is async you don't have to worry about thread-safety issues, only asyncio problems.

thlucas1 commented 3 months ago

Thanks for that explanation.

Unfortunately, the underlying API’s that I am interfacing with do not support async operations. It sounds like I am stuck executing those types of operations with the async_add_executor_job.

Any functions I write in the HA integration can be written with async and decorated with @callback to make them more efficient.

Please correct me if I’m wrong on those assumptions.

bdraco commented 3 months ago

Thanks for that explanation.

Unfortunately, the underlying API’s that I am interfacing with do not support async operations. It sounds like I am stuck executing those types of operations with the async_add_executor_job.

In that case I generally make a new asyncio python package, but thats likely more than you want to do.

Any functions I write in the HA integration can be written with async and decorated with @callback to make them more efficient.

There is a cost to jumping between threads so if you are already in another thread and the work you are doing is thread safe, its best not to jump back into the event loop thread unless the work needs to explicitly do something that would not be thread safe unless it ran in the event loop thread (like working with asyncio primitives)

Please correct me if I’m wrong on those assumptions.

Thrasher2020 commented 3 months ago

Testing right now. Will let you know :)

Thank you.

I can confirm everything is working as before now. Thanks :)

thlucas1 commented 3 months ago

Good to hear - closing the issue. Thanks