From: Daniele Varrazzo Date: Wed, 30 Apr 2025 00:37:23 +0000 (+0200) Subject: fix: don't call str(self) or imported objects in __del__ X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=074d94eaf640df4649826137199be1274f42bd75;p=thirdparty%2Fpsycopg.git fix: don't call str(self) or imported objects in __del__ 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). --- diff --git a/.flake8 b/.flake8 index ed086adf0..79e029388 100644 --- 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 diff --git a/psycopg/psycopg/_connection_base.py b/psycopg/psycopg/_connection_base.py index 7ac6af1a5..b19d60bb4 100644 --- a/psycopg/psycopg/_connection_base.py +++ b/psycopg/psycopg/_connection_base.py @@ -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, ) diff --git a/psycopg/psycopg/_server_cursor.py b/psycopg/psycopg/_server_cursor.py index 3b27d1fc7..84d27cc6a 100644 --- a/psycopg/psycopg/_server_cursor.py +++ b/psycopg/psycopg/_server_cursor.py @@ -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. diff --git a/psycopg/psycopg/_server_cursor_async.py b/psycopg/psycopg/_server_cursor_async.py index 37e0e2621..ff78719b1 100644 --- a/psycopg/psycopg/_server_cursor_async.py +++ b/psycopg/psycopg/_server_cursor_async.py @@ -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. diff --git a/psycopg/psycopg/_server_cursor_base.py b/psycopg/psycopg/_server_cursor_base.py index 4d1fba0d5..6a682bebf 100644 --- a/psycopg/psycopg/_server_cursor_base.py +++ b/psycopg/psycopg/_server_cursor_base.py @@ -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) diff --git a/psycopg/psycopg/pq/pq_ctypes.py b/psycopg/psycopg/pq/pq_ctypes.py index 09d4299f0..53b83bfdd 100644 --- a/psycopg/psycopg/pq/pq_ctypes.py +++ b/psycopg/psycopg/pq/pq_ctypes.py @@ -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. diff --git a/tests/test_connection.py b/tests/test_connection.py index 90aa0ddf3..ad3a262e0 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -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 diff --git a/tests/test_connection_async.py b/tests/test_connection_async.py index 89af8475a..7eb5a27e3 100644 --- a/tests/test_connection_async.py +++ b/tests/test_connection_async.py @@ -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 diff --git a/tests/test_cursor_server.py b/tests/test_cursor_server.py index a295d9aae..64888edba 100644 --- a/tests/test_cursor_server.py +++ b/tests/test_cursor_server.py @@ -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): diff --git a/tests/test_cursor_server_async.py b/tests/test_cursor_server_async.py index eeb198d3b..b175af229 100644 --- a/tests/test_cursor_server_async.py +++ b/tests/test_cursor_server_async.py @@ -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):