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

Fix memory leak in onvif #83006

Merged
merged 9 commits into from Nov 30, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions homeassistant/components/onvif/device.py
Expand Up @@ -11,7 +11,7 @@
import onvif
from onvif import ONVIFCamera
from onvif.exceptions import ONVIFError
from zeep.exceptions import Fault
from zeep.exceptions import Fault, XMLParseError

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import (
Expand Down Expand Up @@ -284,7 +284,7 @@ async def async_get_capabilities(self):
snapshot = media_capabilities and media_capabilities.SnapshotUri

pullpoint = False
with suppress(ONVIFError, Fault, RequestError):
with suppress(ONVIFError, Fault, RequestError, XMLParseError):
pullpoint = await self.events.async_start()

ptz = False
Expand Down
36 changes: 25 additions & 11 deletions homeassistant/components/onvif/event.py
Expand Up @@ -5,6 +5,7 @@
from collections.abc import Callable
from contextlib import suppress
import datetime as dt
from logging import DEBUG, WARNING

from httpx import RemoteProtocolError, TransportError
from onvif import ONVIFCamera, ONVIFService
Expand All @@ -20,7 +21,6 @@

UNHANDLED_TOPICS = set()
SUBSCRIPTION_ERRORS = (
XMLParseError,
Fault,
asyncio.TimeoutError,
TransportError,
Expand Down Expand Up @@ -122,20 +122,32 @@ async def async_restart(self, _now: dt.datetime | None = None) -> None:

if self._subscription:
# Suppressed. The subscription may no longer exist.
with suppress(*SUBSCRIPTION_ERRORS):
try:
await self._subscription.Unsubscribe()
except (XMLParseError, *SUBSCRIPTION_ERRORS) as err:
LOGGER.debug(
"Failed to unsubscribe ONVIF PullPoint subscription for '%s';"
" This is normal if the device restarted: %s",
self.unique_id,
err,
)
self._subscription = None

try:
restarted = await self.async_start()
except SUBSCRIPTION_ERRORS:
except (XMLParseError, *SUBSCRIPTION_ERRORS) as err:
restarted = False

if not restarted:
LOGGER.warning(
"Failed to restart ONVIF PullPoint subscription for '%s'. Retrying",
# Device may not support subscriptions so log at debug level
# when we get an XMLParseError
LOGGER.log(
DEBUG if isinstance(err, XMLParseError) else WARNING,
"Failed to restart ONVIF PullPoint subscription for '%s'; "
"Retrying later: %s",
self.unique_id,
err,
)

if not restarted:
# Try again in a minute
self._unsub_refresh = async_call_later(self.hass, 60, self.async_restart)
elif self._listeners:
Expand All @@ -154,8 +166,7 @@ async def async_renew(self) -> None:
.isoformat(timespec="seconds")
.replace("+00:00", "Z")
)
with suppress(*SUBSCRIPTION_ERRORS):
await self._subscription.Renew(termination_time)
await self._subscription.Renew(termination_time)

def async_schedule_pull(self) -> None:
"""Schedule async_pull_messages to run."""
Expand All @@ -178,8 +189,11 @@ async def async_pull_messages(self, _now: dt.datetime | None = None) -> None:
except RemoteProtocolError:
# Likely a shutdown event, nothing to see here
return
except SUBSCRIPTION_ERRORS as err:
LOGGER.warning(
except (XMLParseError, *SUBSCRIPTION_ERRORS) as err:
# Device may not support subscriptions so log at debug level
# when we get an XMLParseError
LOGGER.log(
DEBUG if isinstance(err, XMLParseError) else WARNING,
"Failed to fetch ONVIF PullPoint subscription messages for '%s': %s",
self.unique_id,
err,
Expand Down