Skip to content

Deprecate implicit closing of drivers and sessions #653

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

Merged
merged 1 commit into from
Feb 1, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 24 additions & 3 deletions neo4j/_async/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,7 +27,11 @@
SessionConfig,
WorkspaceConfig,
)
from ..meta import experimental
from ..meta import (
deprecation_warn,
experimental,
unclosed_resource_warn,
)


class AsyncGraphDatabase:
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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):
Expand Down
11 changes: 0 additions & 11 deletions neo4j/_async/work/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 22 additions & 2 deletions neo4j/_async/work/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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):
Expand Down Expand Up @@ -100,3 +119,4 @@ async def _disconnect(self, sync=False):

async def close(self):
await self._disconnect(sync=True)
self._closed = True
27 changes: 24 additions & 3 deletions neo4j/_sync/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,7 +27,11 @@
SessionConfig,
WorkspaceConfig,
)
from ..meta import experimental
from ..meta import (
deprecation_warn,
experimental,
unclosed_resource_warn,
)


class GraphDatabase:
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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):
Expand Down
11 changes: 0 additions & 11 deletions neo4j/_sync/work/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 22 additions & 2 deletions neo4j/_sync/work/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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):
Expand Down Expand Up @@ -100,3 +119,4 @@ def _disconnect(self, sync=False):

def close(self):
self._disconnect(sync=True)
self._closed = True
13 changes: 13 additions & 0 deletions neo4j/meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)