Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix connection leak when transaction commit/rollback raise an exception #498

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions tests/test_databases.py
Expand Up @@ -575,6 +575,34 @@ async def test_transaction_rollback(database_url):
assert len(results) == 0


@pytest.mark.parametrize("database_url", DATABASE_URLS)
@mysql_versions
masipcat marked this conversation as resolved.
Show resolved Hide resolved
@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
Comment on lines +554 to +570
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aminalaee I think this test it's only valid for postgresql. Is there a way to skip for other databases?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure,

async def test_transaction_commit_serializable(database_url):



@pytest.mark.parametrize("database_url", DATABASE_URLS)
@mysql_versions
@async_adapter
Expand Down