From 0d3c5ae445c9576a6e208e7efd80a8c5c73be269 Mon Sep 17 00:00:00 2001 From: Jordi Masip Date: Mon, 18 Jul 2022 13:16:42 +0200 Subject: [PATCH 1/3] Fix connection leak when transaction commit/rollback raise an exception --- databases/core.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/databases/core.py b/databases/core.py index 2bab6735..c9ffa1d5 100644 --- a/databases/core.py +++ b/databases/core.py @@ -371,15 +371,19 @@ async def commit(self) -> None: async with self._connection._transaction_lock: assert self._connection._transaction_stack[-1] is self self._connection._transaction_stack.pop() - await self._transaction.commit() - await self._connection.__aexit__() + try: + await self._transaction.commit() + finally: + await self._connection.__aexit__() async def rollback(self) -> None: async with self._connection._transaction_lock: assert self._connection._transaction_stack[-1] is self self._connection._transaction_stack.pop() - await self._transaction.rollback() - await self._connection.__aexit__() + try: + await self._transaction.rollback() + finally: + await self._connection.__aexit__() class _EmptyNetloc(str): From 8ff26fd6fe223445374e96a917e0b7b8dd3e711c Mon Sep 17 00:00:00 2001 From: Jordi Masip Date: Fri, 29 Jul 2022 12:12:52 +0200 Subject: [PATCH 2/3] Add test to check transaction doesn't leak on error --- tests/test_databases.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_databases.py b/tests/test_databases.py index e6313e94..54576e4a 100644 --- a/tests/test_databases.py +++ b/tests/test_databases.py @@ -575,6 +575,34 @@ async def test_transaction_rollback(database_url): assert len(results) == 0 +@pytest.mark.parametrize("database_url", DATABASE_URLS) +@mysql_versions +@async_adapter +async def test_transaction_doesnt_leak_when_commit_fails(database_url): + """ + Ensure that transaction doesn't leak the connection when the commit or rollback + fails + """ + + async with Database(database_url) as database: + with pytest.raises(Exception) as excinfo: + async with database.connection() as connection: + await connection.execute( + """ + CREATE TABLE test (id integer PRIMARY KEY INITIALLY DEFERRED); + """ + ) + async with connection.transaction(): + await connection.execute("insert into test (id) values (1)") + await connection.execute("insert into test (id) values (1)") + + # During transaction.commit() postgres will raise this exception: + # asyncpg.exceptions.UniqueViolationError: duplicate key value violates unique constraint "test_pkey" + # DETAIL: Key (id)=(1) already exists. + assert "unique constraint" in str(excinfo.value) + assert connection._connection_counter == 0 + + @pytest.mark.parametrize("database_url", DATABASE_URLS) @mysql_versions @async_adapter From 1a07fa4fe2840703a23345fc29b117b3da84e019 Mon Sep 17 00:00:00 2001 From: Jordi Masip Date: Fri, 29 Jul 2022 12:16:10 +0200 Subject: [PATCH 3/3] Removed @mysql_versions --- tests/test_databases.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_databases.py b/tests/test_databases.py index 4a4eaec4..60f17349 100644 --- a/tests/test_databases.py +++ b/tests/test_databases.py @@ -571,7 +571,6 @@ async def test_transaction_doesnt_leak_when_commit_fails(database_url): @pytest.mark.parametrize("database_url", DATABASE_URLS) -@mysql_versions @async_adapter async def test_transaction_commit_low_level(database_url): """