Deprecate implicit closing of drivers and sessions (#653)

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.

Author: robsdedude

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

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):

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
 

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

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):

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
 

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

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)