]> git.ipfire.org Git - thirdparty/psycopg.git/commitdiff
fix: don't call str(self) or imported objects in __del__ master
authorDaniele Varrazzo <daniele.varrazzo@gmail.com>
Wed, 30 Apr 2025 00:37:23 +0000 (02:37 +0200)
committerDaniele Varrazzo <daniele.varrazzo@gmail.com>
Sat, 5 Jul 2025 17:30:27 +0000 (19:30 +0200)
We easily end up trying to access resources already unavailable in the
objects representation. In Python 3.14 this seems to happen aggressively
(or maybe it's just more visible because testing a dev version).

.flake8
psycopg/psycopg/_connection_base.py
psycopg/psycopg/_server_cursor.py
psycopg/psycopg/_server_cursor_async.py
psycopg/psycopg/_server_cursor_base.py
psycopg/psycopg/pq/pq_ctypes.py
tests/test_connection.py
tests/test_connection_async.py
tests/test_cursor_server.py
tests/test_cursor_server_async.py

diff --git a/.flake8 b/.flake8
index ed086adf0efc6701ea866b825acfcc8f79a90e06..79e029388048c5ca75725c6bd2ad3353bacc472f 100644 (file)
--- a/.flake8
+++ b/.flake8
@@ -12,7 +12,6 @@ per-file-ignores =
     psycopg/psycopg/errors.py: E125, E128, E302
 
     # Allow concatenated string literals from async_to_sync
-    psycopg/psycopg/_server_cursor.py: E501
     psycopg_pool/psycopg_pool/pool.py: E501
 
     # Pytest's importorskip() getting in the way
index 53d06cc4f477eaba87b9b688be7e1b0fe04d15c3..1a2b777afa7a4a1d1c10ca06fa76be649e009819 100644 (file)
@@ -8,7 +8,7 @@ from __future__ import annotations
 
 import sys
 import logging
-from typing import TYPE_CHECKING, Generic, NamedTuple, TypeAlias
+from typing import TYPE_CHECKING, Any, Generic, NamedTuple, TypeAlias
 from weakref import ReferenceType, ref
 from warnings import warn
 from functools import partial
@@ -135,7 +135,7 @@ class BaseConnection(Generic[Row]):
         self._deferrable: bool | None = None
         self._begin_statement = b""
 
-    def __del__(self) -> None:
+    def __del__(self, __warn: Any = warn) -> None:
         # If fails on connection we might not have this attribute yet
         if not hasattr(self, "pgconn"):
             return
@@ -148,9 +148,9 @@ class BaseConnection(Generic[Row]):
         if hasattr(self, "_pool"):
             return
 
-        warn(
-            f"connection {self} was deleted while still open."
-            " Please use 'with' or '.close()' to close the connection",
+        __warn(
+            f"{object.__repr__(self)} was deleted while still open."
+            " Please use 'with' or '.close()' to close the connection properly",
             ResourceWarning,
         )
 
index cfddfd97c0eb786c4e873450861b3d9879cfda6b..6eefb851092ead84b88a6b247717920415a555f1 100644 (file)
@@ -10,7 +10,6 @@ psycopg server-side cursor (sync).
 from __future__ import annotations
 
 from typing import TYPE_CHECKING, Any, overload
-from warnings import warn
 from collections.abc import Iterable
 
 from . import errors as e
@@ -63,13 +62,6 @@ class ServerCursor(ServerCursorMixin["Connection[Any]", Row], Cursor[Row]):
         )
         ServerCursorMixin.__init__(self, name, scrollable, withhold)
 
-    def __del__(self) -> None:
-        if not self.closed:
-            warn(
-                f"the server-side cursor {self} was deleted while still open. Please use 'with' or '.close()' to close the cursor properly",
-                ResourceWarning,
-            )
-
     def close(self) -> None:
         """
         Close the current cursor and free associated resources.
index 41628fe74cbf7fb015160bf277e40a587dffa26a..a37084b321bbe9dc120ffb35cb5f00533e085b78 100644 (file)
@@ -7,7 +7,6 @@ psycopg server-side cursor (async).
 from __future__ import annotations
 
 from typing import TYPE_CHECKING, Any, overload
-from warnings import warn
 from collections.abc import Iterable
 
 from . import errors as e
@@ -62,14 +61,6 @@ class AsyncServerCursor(
         )
         ServerCursorMixin.__init__(self, name, scrollable, withhold)
 
-    def __del__(self) -> None:
-        if not self.closed:
-            warn(
-                f"the server-side cursor {self} was deleted while still open."
-                " Please use 'with' or '.close()' to close the cursor properly",
-                ResourceWarning,
-            )
-
     async def close(self) -> None:
         """
         Close the current cursor and free associated resources.
index fef6f9070edf5c3e3edd49794e9aa6633dff6f2a..b6ca6843f75fb462931770c9716f131f4af959ad 100644 (file)
@@ -6,6 +6,9 @@ psycopg server-side cursor base objects.
 
 from __future__ import annotations
 
+from typing import Any
+from warnings import warn
+
 from . import errors as e
 from . import pq, sql
 from .abc import ConnectionType, Params, PQGen, Query
@@ -44,6 +47,16 @@ class ServerCursorMixin(BaseCursor[ConnectionType, Row]):
         self._iter_rows: list[Row] | None = None
         self._page_pos = 0
 
+    def __del__(self, __warn: Any = warn) -> None:
+        if self.closed:
+            return
+
+        __warn(
+            f"{object.__repr__(self)} was deleted while still open."
+            " Please use 'with' or '.close()' to close the cursor properly",
+            ResourceWarning,
+        )
+
     def __repr__(self) -> str:
         # Insert the name as the second word
         parts = super().__repr__().split(None, 1)
index 7c16451e3cf1b9796a93f8a84b5f3658ca90ed94..cf9684f5e2b5c4ec450bb634d9fc8aa0f6fb3fe0 100644 (file)
@@ -27,9 +27,6 @@ from .. import errors as e
 from .misc import ConninfoOption, PGnotify, PGresAttDesc, _clean_error_message
 from .misc import connection_summary
 from ._enums import ConnStatus, ExecStatus, Format, Trace
-
-# Imported locally to call them from __del__ methods
-from ._pq_ctypes import PQcancelFinish, PQclear, PQfinish, PQfreeCancel, PQstatus
 from .._encodings import pg2pyenc
 
 if TYPE_CHECKING:
@@ -41,6 +38,10 @@ logger = logging.getLogger("psycopg")
 
 OK = ConnStatus.OK
 
+# note_on_del: functions called on __del__ are imported as local values to
+# avoid warnings on interpreter shutdown in case the module is gc'd before the
+# object is destroyed.
+
 
 def version() -> int:
     """Return the version number of the libpq currently loaded.
@@ -95,10 +96,10 @@ class PGconn:
 
         self._procpid = getpid()
 
-    def __del__(self) -> None:
+    def __del__(self, __getpid: Callable[[], int] = getpid) -> None:
         # Close the connection only if it was created in this process,
         # not if this object is being GC'd after fork.
-        if getpid() == self._procpid:
+        if __getpid() == self._procpid:  # see note_on_del
             self.finish()
 
     def __repr__(self) -> str:
@@ -125,10 +126,10 @@ class PGconn:
     def connect_poll(self) -> int:
         return self._call_int(impl.PQconnectPoll)
 
-    def finish(self) -> None:
+    def finish(self, __PQfinish: Any = impl.PQfinish) -> None:
         self._pgconn_ptr, p = None, self._pgconn_ptr
         if p:
-            PQfinish(p)
+            __PQfinish(p)
 
     @property
     def pgconn_ptr(self) -> int | None:
@@ -206,8 +207,8 @@ class PGconn:
         return self._call_bytes(impl.PQoptions)
 
     @property
-    def status(self) -> int:
-        return PQstatus(self._pgconn_ptr)
+    def status(self, __PQstatus: Any = impl.PQstatus) -> int:
+        return __PQstatus(self._pgconn_ptr)
 
     @property
     def transaction_status(self) -> int:
@@ -828,13 +829,16 @@ class PGresult:
 
     def __repr__(self) -> str:
         cls = f"{self.__class__.__module__}.{self.__class__.__qualname__}"
-        status = ExecStatus(self.status)
-        return f"<{cls} [{status.name}] at 0x{id(self):x}>"
+        try:
+            status = ExecStatus(self.status).name
+        except ValueError:
+            status = f"{self.status} (status unknown)"
+        return f"<{cls} [{status}] at 0x{id(self):x}>"
 
-    def clear(self) -> None:
+    def clear(self, __PQclear: Any = impl.PQclear) -> None:
         self._pgresult_ptr, p = None, self._pgresult_ptr
         if p:
-            PQclear(p)
+            __PQclear(p)  # see note_on_del
 
     @property
     def pgresult_ptr(self) -> int | None:
@@ -1000,7 +1004,7 @@ class PGcancelConn:
         self._ensure_pgcancelconn()
         impl.PQcancelReset(self.pgcancelconn_ptr)
 
-    def finish(self) -> None:
+    def finish(self, __PQcancelFinish: Any = impl.PQcancelFinish) -> None:
         """
         Free the data structure created by `PQcancelCreate()`.
 
@@ -1010,7 +1014,7 @@ class PGcancelConn:
         """
         self.pgcancelconn_ptr, p = None, self.pgcancelconn_ptr
         if p:
-            PQcancelFinish(p)
+            __PQcancelFinish(p)  # see note_on_del
 
     def _ensure_pgcancelconn(self) -> None:
         if not self.pgcancelconn_ptr:
@@ -1032,7 +1036,7 @@ class PGcancel:
     def __del__(self) -> None:
         self.free()
 
-    def free(self) -> None:
+    def free(self, __PQfreeCancel: Any = impl.PQfreeCancel) -> None:
         """
         Free the data structure created by :pq:`PQgetCancel()`.
 
@@ -1042,7 +1046,7 @@ class PGcancel:
         """
         self.pgcancel_ptr, p = None, self.pgcancel_ptr
         if p:
-            PQfreeCancel(p)
+            __PQfreeCancel(p)  # see note_on_del
 
     def cancel(self) -> None:
         """Requests that the server abandon processing of the current command.
index 90aa0ddf3fcb73b1f53af15d5d75784a8507013e..ad3a262e0dbb60a55abe9caa909d5caaf32ea70e 100644 (file)
@@ -197,13 +197,13 @@ def test_connection_warn_close(conn_cls, dsn, recwarn, gc_collect):
     conn = conn_cls.connect(dsn)
     del conn
     gc_collect()
-    assert "IDLE" in str(recwarn.pop(ResourceWarning).message)
+    assert conn_cls.__name__ in str(recwarn.pop(ResourceWarning).message)
 
     conn = conn_cls.connect(dsn)
     conn.execute("select 1")
     del conn
     gc_collect()
-    assert "INTRANS" in str(recwarn.pop(ResourceWarning).message)
+    assert conn_cls.__name__ in str(recwarn.pop(ResourceWarning).message)
 
     conn = conn_cls.connect(dsn)
     try:
@@ -212,7 +212,7 @@ def test_connection_warn_close(conn_cls, dsn, recwarn, gc_collect):
         pass
     del conn
     gc_collect()
-    assert "INERROR" in str(recwarn.pop(ResourceWarning).message)
+    assert conn_cls.__name__ in str(recwarn.pop(ResourceWarning).message)
 
     with conn_cls.connect(dsn) as conn:
         pass
index 89af8475a5f284cea9175126c2ecf57e01c83bee..7eb5a27e30ab2fc892766f5ee6642b28249786ce 100644 (file)
@@ -192,13 +192,13 @@ async def test_connection_warn_close(aconn_cls, dsn, recwarn, gc_collect):
     conn = await aconn_cls.connect(dsn)
     del conn
     gc_collect()
-    assert "IDLE" in str(recwarn.pop(ResourceWarning).message)
+    assert aconn_cls.__name__ in str(recwarn.pop(ResourceWarning).message)
 
     conn = await aconn_cls.connect(dsn)
     await conn.execute("select 1")
     del conn
     gc_collect()
-    assert "INTRANS" in str(recwarn.pop(ResourceWarning).message)
+    assert aconn_cls.__name__ in str(recwarn.pop(ResourceWarning).message)
 
     conn = await aconn_cls.connect(dsn)
     try:
@@ -207,7 +207,7 @@ async def test_connection_warn_close(aconn_cls, dsn, recwarn, gc_collect):
         pass
     del conn
     gc_collect()
-    assert "INERROR" in str(recwarn.pop(ResourceWarning).message)
+    assert aconn_cls.__name__ in str(recwarn.pop(ResourceWarning).message)
 
     async with await aconn_cls.connect(dsn) as conn:
         pass
index 13b08006b1a1698b780ee83a7c045254f41440ac..7bcc305290ad69d51d372c02d2a24a7f9abd5092 100644 (file)
@@ -274,7 +274,9 @@ def test_warn_close(conn, recwarn, gc_collect):
     cur.execute("select generate_series(1, 10) as bar")
     del cur
     gc_collect()
-    assert ".close()" in str(recwarn.pop(ResourceWarning).message)
+    msg = str(recwarn.pop(ResourceWarning).message)
+    assert conn.server_cursor_factory.__name__ in msg
+    assert ".close()" in msg
 
 
 def test_execute_reuse(conn):
index 725c4b64bb8a34b29c3c6842b7736e0f0b1051e8..f3c57d56b7bb584b79431616e40c09ed903fdfd2 100644 (file)
@@ -280,7 +280,9 @@ async def test_warn_close(aconn, recwarn, gc_collect):
     await cur.execute("select generate_series(1, 10) as bar")
     del cur
     gc_collect()
-    assert ".close()" in str(recwarn.pop(ResourceWarning).message)
+    msg = str(recwarn.pop(ResourceWarning).message)
+    assert aconn.server_cursor_factory.__name__ in msg
+    assert ".close()" in msg
 
 
 async def test_execute_reuse(aconn):