From e0162a8e85967d262f53c1eaa435f8af6ff9909b Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Tue, 1 Feb 2022 18:15:15 +0100 Subject: [PATCH] Deprecate implicit closing of drivers and sessions Deprecated closing of driver and session objects in their destructor. This behaviour is non-deterministic as there is no guarantee that the destructor will ever be called. A `ResourceWarning` is emitted instead. --- CHANGELOG.md | 5 +++++ neo4j/_async/driver.py | 27 ++++++++++++++++++++++++--- neo4j/_async/work/session.py | 11 ----------- neo4j/_async/work/workspace.py | 24 ++++++++++++++++++++++-- neo4j/_sync/driver.py | 27 ++++++++++++++++++++++++--- neo4j/_sync/work/session.py | 11 ----------- neo4j/_sync/work/workspace.py | 24 ++++++++++++++++++++++-- neo4j/meta.py | 13 +++++++++++++ 8 files changed, 110 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aa94c42..4bd4f126 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,11 @@ Use `Session.last_bookmarks` instead. - `neo4j.Bookmark` was deprecated. Use `neo4j.Bookmarks` instead. +- Deprecated closing of driver and session objects in their destructor. + This behaviour is non-deterministic as there is no guarantee that the + destructor will ever be called. A `ResourceWarning` is emitted instead. + Make sure to configure Python to output those warnings when developing your + application locally (it does not by default). ## Version 4.4 diff --git a/neo4j/_async/driver.py b/neo4j/_async/driver.py index 0284477a..7751329d 100644 --- a/neo4j/_async/driver.py +++ b/neo4j/_async/driver.py @@ -16,6 +16,8 @@ # limitations under the License. +import asyncio + from .._async_compat.util import AsyncUtil from ..addressing import Address from ..api import READ_ACCESS @@ -25,7 +27,11 @@ SessionConfig, WorkspaceConfig, ) -from ..meta import experimental +from ..meta import ( + deprecation_warn, + experimental, + unclosed_resource_warn, +) class AsyncGraphDatabase: @@ -189,6 +195,9 @@ class AsyncDriver: #: Connection pool _pool = None + #: Flag if the driver has been closed + _closed = False + def __init__(self, pool): assert pool is not None self._pool = pool @@ -200,8 +209,19 @@ async def __aexit__(self, exc_type, exc_value, traceback): await self.close() def __del__(self): - if not AsyncUtil.is_async_code: - self.close() + if not self._closed: + unclosed_resource_warn(self) + # TODO: 6.0 - remove this + if not self._closed: + if not AsyncUtil.is_async_code: + deprecation_warn( + "Relying on AsyncDriver's destructor to close the session " + "is deprecated. Please make sure to close the session. " + "Use it as a context (`with` statement) or make sure to " + "call `.close()` explicitly. Future versions of the " + "driver will not close drivers automatically." + ) + self.close() @property def encrypted(self): @@ -225,6 +245,7 @@ async def close(self): """ Shut down, closing any open connections in the pool. """ await self._pool.close() + self._closed = True @experimental("The configuration may change in the future.") async def verify_connectivity(self, **config): diff --git a/neo4j/_async/work/session.py b/neo4j/_async/work/session.py index 5f608e57..7e3da937 100644 --- a/neo4j/_async/work/session.py +++ b/neo4j/_async/work/session.py @@ -84,22 +84,11 @@ class AsyncSession(AsyncWorkspace): # The state this session is in. _state_failed = False - # Session have been properly closed. - _closed = False - def __init__(self, pool, session_config): super().__init__(pool, session_config) assert isinstance(session_config, SessionConfig) self._bookmarks = self._prepare_bookmarks(session_config.bookmarks) - def __del__(self): - if asyncio.iscoroutinefunction(self.close): - return - try: - self.close() - except (OSError, ServiceUnavailable, SessionExpired): - pass - async def __aenter__(self): return self diff --git a/neo4j/_async/work/workspace.py b/neo4j/_async/work/workspace.py index 5dee191a..b4743025 100644 --- a/neo4j/_async/work/workspace.py +++ b/neo4j/_async/work/workspace.py @@ -19,7 +19,14 @@ import asyncio from ...conf import WorkspaceConfig -from ...exceptions import ServiceUnavailable +from ...exceptions import ( + ServiceUnavailable, + SessionExpired, +) +from ...meta import ( + deprecation_warn, + unclosed_resource_warn, +) from ..io import AsyncNeo4jPool @@ -34,13 +41,25 @@ def __init__(self, pool, config): # Sessions are supposed to cache the database on which to operate. self._cached_database = False self._bookmarks = None + # Workspace has been closed. + self._closed = False def __del__(self): + if not self._closed: + unclosed_resource_warn(self) + # TODO: 6.0 - remove this if asyncio.iscoroutinefunction(self.close): return try: + deprecation_warn( + "Relying on AsyncSession's destructor to close the session " + "is deprecated. Please make sure to close the session. Use it " + "as a context (`with` statement) or make sure to call " + "`.close()` explicitly. Future versions of the driver will " + "not close sessions automatically." + ) self.close() - except OSError: + except (OSError, ServiceUnavailable, SessionExpired): pass async def __aenter__(self): @@ -100,3 +119,4 @@ async def _disconnect(self, sync=False): async def close(self): await self._disconnect(sync=True) + self._closed = True diff --git a/neo4j/_sync/driver.py b/neo4j/_sync/driver.py index 4afced35..4ceaf5a3 100644 --- a/neo4j/_sync/driver.py +++ b/neo4j/_sync/driver.py @@ -16,6 +16,8 @@ # limitations under the License. +import asyncio + from .._async_compat.util import Util from ..addressing import Address from ..api import READ_ACCESS @@ -25,7 +27,11 @@ SessionConfig, WorkspaceConfig, ) -from ..meta import experimental +from ..meta import ( + deprecation_warn, + experimental, + unclosed_resource_warn, +) class GraphDatabase: @@ -189,6 +195,9 @@ class Driver: #: Connection pool _pool = None + #: Flag if the driver has been closed + _closed = False + def __init__(self, pool): assert pool is not None self._pool = pool @@ -200,8 +209,19 @@ def __exit__(self, exc_type, exc_value, traceback): self.close() def __del__(self): - if not Util.is_async_code: - self.close() + if not self._closed: + unclosed_resource_warn(self) + # TODO: 6.0 - remove this + if not self._closed: + if not Util.is_async_code: + deprecation_warn( + "Relying on Driver's destructor to close the session " + "is deprecated. Please make sure to close the session. " + "Use it as a context (`with` statement) or make sure to " + "call `.close()` explicitly. Future versions of the " + "driver will not close drivers automatically." + ) + self.close() @property def encrypted(self): @@ -225,6 +245,7 @@ def close(self): """ Shut down, closing any open connections in the pool. """ self._pool.close() + self._closed = True @experimental("The configuration may change in the future.") def verify_connectivity(self, **config): diff --git a/neo4j/_sync/work/session.py b/neo4j/_sync/work/session.py index 6ef573fb..f188f952 100644 --- a/neo4j/_sync/work/session.py +++ b/neo4j/_sync/work/session.py @@ -84,22 +84,11 @@ class Session(Workspace): # The state this session is in. _state_failed = False - # Session have been properly closed. - _closed = False - def __init__(self, pool, session_config): super().__init__(pool, session_config) assert isinstance(session_config, SessionConfig) self._bookmarks = self._prepare_bookmarks(session_config.bookmarks) - def __del__(self): - if asyncio.iscoroutinefunction(self.close): - return - try: - self.close() - except (OSError, ServiceUnavailable, SessionExpired): - pass - def __enter__(self): return self diff --git a/neo4j/_sync/work/workspace.py b/neo4j/_sync/work/workspace.py index 3ed50ad2..aa53d535 100644 --- a/neo4j/_sync/work/workspace.py +++ b/neo4j/_sync/work/workspace.py @@ -19,7 +19,14 @@ import asyncio from ...conf import WorkspaceConfig -from ...exceptions import ServiceUnavailable +from ...exceptions import ( + ServiceUnavailable, + SessionExpired, +) +from ...meta import ( + deprecation_warn, + unclosed_resource_warn, +) from ..io import Neo4jPool @@ -34,13 +41,25 @@ def __init__(self, pool, config): # Sessions are supposed to cache the database on which to operate. self._cached_database = False self._bookmarks = None + # Workspace has been closed. + self._closed = False def __del__(self): + if not self._closed: + unclosed_resource_warn(self) + # TODO: 6.0 - remove this if asyncio.iscoroutinefunction(self.close): return try: + deprecation_warn( + "Relying on Session's destructor to close the session " + "is deprecated. Please make sure to close the session. Use it " + "as a context (`with` statement) or make sure to call " + "`.close()` explicitly. Future versions of the driver will " + "not close sessions automatically." + ) self.close() - except OSError: + except (OSError, ServiceUnavailable, SessionExpired): pass def __enter__(self): @@ -100,3 +119,4 @@ def _disconnect(self, sync=False): def close(self): self._disconnect(sync=True) + self._closed = True diff --git a/neo4j/meta.py b/neo4j/meta.py index 6c06c6a8..2b497249 100644 --- a/neo4j/meta.py +++ b/neo4j/meta.py @@ -96,3 +96,16 @@ def f_(*args, **kwargs): return f(*args, **kwargs) return f_ return f__ + + +def unclosed_resource_warn(obj): + import tracemalloc + from warnings import warn + msg = f"Unclosed {obj!r}." + trace = tracemalloc.get_object_traceback(obj) + if trace: + msg += "\nObject allocated at (most recent call last):\n" + msg += "\n".join(trace.format()) + else: + msg += "\nEnable tracemalloc to get the object allocation traceback." + warn(msg, ResourceWarning, stacklevel=2, source=obj)