#37000: cursor_iter relies on GC for server-side cursor cleanup, causing
transaction abort after savepoint rollback
-------------------------------------+-------------------------------------
Reporter: Ratskó László | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: iterator, server- | Triage Stage: Accepted
side-cursor, savepoint, psycopg3 |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I was able to reproduce with the following test
{{{#!diff
diff --git a/tests/backends/postgresql/test_server_side_cursors.py
b/tests/backends/postgresql/test_server_side_cursors.py
index 9a6457cce6..900a2dc229 100644
--- a/tests/backends/postgresql/test_server_side_cursors.py
+++ b/tests/backends/postgresql/test_server_side_cursors.py
@@ -1,9 +1,10 @@
import operator
+import gc
import unittest
from collections import namedtuple
from contextlib import contextmanager
-from django.db import connection, models
+from django.db import connection, models, transaction
from django.db.utils import ProgrammingError
from django.test import TestCase
from django.test.utils import garbage_collect
@@ -154,3 +155,12 @@ class ServerSideCursorsPostgres(TestCase):
# most likely need to be adapted.
with self.assertRaises(ProgrammingError):
perform_query()
+
+ def test_transaction_cursor_closing(self):
+ with self.assertRaises(ValueError), transaction.atomic():
+ persons = Person.objects.iterator()
+ next(persons)
+ raise ValueError
+ del persons
+ gc.collect()
+ list(Person.objects.all())
}}}
Without the last query we still get presented with a resource warning
{{{#!python
Exception ignored while closing generator <generator object cursor_iter at
0x7ffb1b97fab0>:
Traceback (most recent call last):
File "/django/source/django/db/models/sql/compiler.py", line 2266, in
cursor_iter
cursor.close()
File "/usr/local/lib/python3.14/site-
packages/psycopg/_server_cursor.py", line 73, in close
self._conn.wait(self._close_gen())
File "/usr/local/lib/python3.14/site-packages/psycopg/connection.py",
line 484, in wait
return waiting.wait(gen, self.pgconn.socket, interval=interval)
File "psycopg_binary/_psycopg/waiting.pyx", line 241, in
psycopg_binary._psycopg.wait_c
File "/usr/local/lib/python3.14/site-
packages/psycopg/_server_cursor_base.py", line 163, in _close_gen
yield from self._conn._exec_command(query)
File "/usr/local/lib/python3.14/site-
packages/psycopg/_connection_base.py", line 483, in _exec_command
raise e.error_from_result(result, encoding=self.pgconn._encoding)
psycopg.errors.InvalidCursorName: cursor
"_django_curs_140716552528704_sync_1" does not exist
Exception ignored while calling deallocator <function
ServerCursorMixin.__del__ at 0x7ffb1c113060>:
Traceback (most recent call last):
File "/usr/local/lib/python3.14/site-
packages/psycopg/_server_cursor_base.py", line 55, in __del__
__warn(
ResourceWarning: <django.db.backends.postgresql.base.ServerSideCursor
object at 0x55b16299aac0> was deleted while still open. Please use 'with'
or '.close()' to close the cursor properly
}}}
The approach in [https://github.com/django/django/pull/20975/ the Claude
generated MR] has merit but it's too naive in the sense that it tries to
close all server side cursors on each savepoint rollbacks but these can be
nested and we likely want to keep weakrefs to cursors as to avoid memory
leaks.
An ideal solution would weak track cursors by savepoint ID and only do so
in nested transactions. I'll note that even if we do so failing to consume
iterators returned by `QuerySet.iterator` entirely will always incur a
`ResourceWarning` (transaction or not) so maybe we should also (or
instead) adjust
[https://docs.djangoproject.com/en/6.0/ref/models/querysets/#django.db.models.query.QuerySet.iterator
the documentation] to mention that the following pattern should be used
instead?
{{{#!python
import contextlib
def export_items():
try:
with (
transaction.atomic(),
contextlib.closing(Item.objects.iterator()) as items
):
for item in items:
if item.value == "bad":
raise ValueError("Export failed")
process(item)
except ValueError:
Item.objects.create(value="error logged")
}}}
to enforce explicit closing of iterators? Maybe we should even have the
object returned by `QuerySet.iterator` be a context closing of itself so
we can document
{{{#!python
def export_items():
try:
with (
transaction.atomic(),
Item.objects.iterator() as items,
):
for item in items:
if item.value == "bad":
raise ValueError("Export failed")
process(item)
except ValueError:
Item.objects.create(value="error logged")
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37000#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019d1d2a10df-fde0b134-23e0-48ad-a776-2314b1d6cf1c-000000%40eu-central-1.amazonses.com.
