]> git.ipfire.org Git - thirdparty/psycopg.git/commitdiff
fix: don't call str(self) or imported objects in __del__
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:29:08 +0000 (19:29 +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 7ac6af1a5ccb233ff7393a7225947c744e033905..b19d60bb41954b9128706ed4a0396d69c85f0211 100644 (file)
@@ -8,7 +8,7 @@ from __future__ import annotations
 
 import sys
 import logging
-from typing import TYPE_CHECKING, Callable, Generic, NamedTuple
+from typing import TYPE_CHECKING, Any, Callable, Generic, NamedTuple
 from weakref import ReferenceType, ref
 from warnings import warn
 from functools import partial
@@ -133,7 +133,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
@@ -146,9 +146,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 3b27d1fc736b556625021b2e118dea47536a0daf..84d27cc6a7a923ec31d78366e7dee148f3db4387 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, Iterator
 
 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 37e0e2621e0abff08d9ba310bf07fec9d1f7bd51..ff78719b10b854c66f4277b60c036cc553a23f54 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 AsyncIterator, 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 4d1fba0d50a585426a567170eb7017c40d663358..6a682bebf0ec510c0587cea70a3f60c3f27e32e6 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
@@ -38,6 +41,16 @@ class ServerCursorMixin(BaseCursor[ConnectionType, Row]):
         self.itersize: int = DEFAULT_ITERSIZE
         self._format = TEXT
 
+    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 09d4299f027da92568a0d87e744ddbaa9770aef0..53b83bfddbdc650a558d9c1c028425e6c54704a2 100644 (file)
@@ -26,9 +26,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:
@@ -40,6 +37,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.
@@ -94,10 +95,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:
@@ -124,10 +125,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:
@@ -205,8 +206,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:
@@ -827,13 +828,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:
@@ -999,7 +1003,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()`.
 
@@ -1009,7 +1013,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:
@@ -1031,7 +1035,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()`.
 
@@ -1041,7 +1045,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 a295d9aae1238878149ea9cd2017b672c6488cec..64888edba6952a0d13633334625bd69a0deb0983 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 eeb198d3bce76cc46c71c64a87eaf9d6e7629f39..b175af2299a6cb52216e839c39113f6daf66389a 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):