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

Use the shared zeroconf instance when attempting to create another Zeroconf instance #38744

Merged
merged 9 commits into from Aug 12, 2020

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Aug 11, 2020

Type of change

Multiple Zeroconf instances keep causing us problems and reports of
high cpu usage. Log when they are created.

2020-08-11 02:42:56 WARNING (MainThread) [homeassistant.helpers.frame] Detected integration that created another Zeroconf instance. Please use the shared Zeroconf via homeassistant.components.zeroconf.async_get_instance(). Please report issue to the custom component author for burncpu using this method at custom_components/burncpu/__init__.py, line 22: zeroconf.Zeroconf()

#38248 (comment)
https://community.home-assistant.io/t/high-cpu-usage-after-0-113/214655/30

If an integration attempts to create another instance, we now provide them the shared
instance, and log.

Note: esphome is the last known integration in core that still does this and it is fixed here #38747

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@probot-home-assistant
Copy link

Hey there @Kane610, mind taking a look at this pull request as its been labeled with an integration (zeroconf) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@Kane610
Copy link
Member

Kane610 commented Aug 11, 2020

Is it possible to get what file or class initiates this second zeroconf instance?

@bdraco
Copy link
Member Author

bdraco commented Aug 11, 2020

Is it possible to get what file or class initiates this second zeroconf instance?

The warning contains that. I'll make sure to keep that when I rework it.

@bdraco bdraco marked this pull request as draft August 11, 2020 12:34
@bdraco bdraco changed the title Warn when creating multiple Zeroconf instances Use the shared zeroconf instance when attempting to create another Zeroconf instance Aug 11, 2020
@bdraco
Copy link
Member Author

bdraco commented Aug 11, 2020

If the code runs in the executor we won't have enough stack to figure out which integration

homeassistant.helpers.frame.MissingIntegrationFrame

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/aioesphomeapi/host_resolver.py", line 56, in resolve_host
    zc = zeroconf.Zeroconf()
  File "/usr/src/homeassistant/homeassistant/components/zeroconf/usage.py", line 12, in new_zeroconf_new
    report(
  File "/usr/src/homeassistant/homeassistant/helpers/frame.py", line 61, in report
    raise RuntimeError(f"Detected code that {what}. Please report this issue.")
RuntimeError: Detected code that attempted to create another Zeroconf instance. Please use the shared Zeroconf via await homeassistant.components.zeroconf.async_get_instance(hass). Please report this issue.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/aioesphomeapi/util.py", line 59, in resolve_ip_address
    return await eventloop.run_in_executor(None, resolve_host, host), port
  File "/usr/local/lib/python3.8/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.8/site-packages/aioesphomeapi/host_resolver.py", line 58, in resolve_host

Probably better to log the stack in that case instead

@bdraco
Copy link
Member Author

bdraco commented Aug 11, 2020

Here is what we report when we can't figure out the integration. I couldn't figure out a way to get the integration when the code is running in the executor since we don't have the stack that created the executor job so I figured a traceback was the best for now

2020-08-11 15:33:42 WARNING (SyncWorker_0) [homeassistant.components.zeroconf.usage] Detected code that attempted to create another Zeroconf instance. Please use the shared Zeroconf via await homeassistant.components.zeroconf.async_get_instance(hass). Please report this issue.
Stack (most recent call last):
  File "/usr/local/lib/python3.8/threading.py", line 890, in _bootstrap
    self._bootstrap_inner()
  File "/usr/local/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/usr/local/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/lib/python3.8/concurrent/futures/thread.py", line 80, in _worker
    work_item.run()
  File "/usr/local/lib/python3.8/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.8/site-packages/aioesphomeapi/host_resolver.py", line 56, in resolve_host
    zc = zeroconf.Zeroconf()
  File "/usr/src/homeassistant/homeassistant/components/zeroconf/usage.py", line 20, in new_zeroconf_new
    _report(
  File "/usr/src/homeassistant/homeassistant/components/zeroconf/usage.py", line 41, in _report
    _LOGGER.warning(
2020-08-11 15:33:42 WARNING (SyncWorker_4) [homeassistant.components.zeroconf.usage] Detected code that attempted to create another Zeroconf instance. Please use the shared Zeroconf via await homeassistant.components.zeroconf.async_get_instance(hass). Please report this issue.
Stack (most recent call last):
  File "/usr/local/lib/python3.8/threading.py", line 890, in _bootstrap
    self._bootstrap_inner()
  File "/usr/local/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/usr/local/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/lib/python3.8/concurrent/futures/thread.py", line 80, in _worker
    work_item.run()
  File "/usr/local/lib/python3.8/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.8/site-packages/aioesphomeapi/host_resolver.py", line 56, in resolve_host
    zc = zeroconf.Zeroconf()
  File "/usr/src/homeassistant/homeassistant/components/zeroconf/usage.py", line 20, in new_zeroconf_new
    _report(
  File "/usr/src/homeassistant/homeassistant/components/zeroconf/usage.py", line 41, in _report
    _LOGGER.warning(

@bdraco bdraco marked this pull request as ready for review August 11, 2020 15:37
@bdraco bdraco marked this pull request as draft August 11, 2020 16:45
@bdraco
Copy link
Member Author

bdraco commented Aug 11, 2020

Thread 263 (idle): "zeroconf-Engine-263"
    run (zeroconf/__init__.py:1309)
    _bootstrap_inner (threading.py:932)
    _bootstrap (threading.py:890)
Thread 264 (idle): "zeroconf-Reaper_264"
    wait (threading.py:306)
    wait (zeroconf/__init__.py:2360)
    run (zeroconf/__init__.py:1422)
    _bootstrap_inner (threading.py:932)
    _bootstrap (threading.py:890)
Thread 265 (idle): "zeroconf-Engine-265"
    run (zeroconf/__init__.py:1309)
    _bootstrap_inner (threading.py:932)
    _bootstrap (threading.py:890)
Thread 266 (idle): "zeroconf-Reaper_266"
    wait (threading.py:306)
    wait (zeroconf/__init__.py:2360)
    run (zeroconf/__init__.py:1422)
    _bootstrap_inner (threading.py:932)
    _bootstrap (threading.py:890)

Still seeing multiple instances

@bdraco bdraco marked this pull request as ready for review August 11, 2020 17:06


def install_multiple_zeroconf_warning() -> None:
"""Wrap the Zeroconf class to warn if multiple instances are detected."""
def install_multiple_zeroconf_catcher(zc) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

We should make sure that we don't call this method in any of the tests. I think that we should put something like this in tests/conftest.py:

from homeassistant.components.zeroconf import usage

usage.orig_install_multiple_zeroconf_catcher = usage.install_multiple_zeroconf_catcher
usage.install_multiple_zeroconf_catcher = lambda zc: None

And then write the tests against the orig_ function.

Copy link
Member Author

Choose a reason for hiding this comment

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

I re-arranged the tests, added conftest, and test_usage. Hopefully that was what you were looking for. 🤞

@balloob balloob merged commit 444df4a into home-assistant:dev Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.113.0 - Zeroconf errors
4 participants