-
-
Notifications
You must be signed in to change notification settings - Fork 447
Description
Describe the bug
update_or_create has a race condition when creating new rows. Two concurrent calls on the same non-existent key both see "no row exists" and both proceed to INSERT. The losing caller's INSERT fails with IntegrityError, which is caught and retried via GET, but GET just fetches the row without applying defaults. The caller neither created nor updated, despite the method name.
This happens because the transaction ends before _create_or_get is called. Both transactions complete their SELECT FOR UPDATE (finding nothing), then race on the subsequent INSERT.
# Current implementation, trimmed for clarity
# https://github.com/tortoise/tortoise-orm/blob/a7259172a034208884f077a8ffcc5d49d599f3d9/tortoise/models.py#L1181
async with in_transaction(connection_name=db.connection_name) as connection:
instance = await cls.select_for_update().using_db(connection).get_or_none(**kwargs)
if instance:
await instance.update_from_dict(defaults).save(using_db=connection)
return instance, False
return await cls._create_or_get(db, defaults, **kwargs)To Reproduce
This is what would trigger the race in a real application, but the timing window is narrow and hard to hit in practice:
import asyncio
from tortoise import Tortoise, fields
from tortoise.models import Model
class Item(Model):
id = fields.IntField(primary_key=True)
name = fields.CharField(max_length=100, unique=True)
value = fields.CharField(max_length=100)
async def main():
await Tortoise.init(db_url="postgres://...", modules={"models": [__name__]})
await Tortoise.generate_schemas()
# Concurrent update_or_create on same key
results = await asyncio.gather(
Item.update_or_create(name="test", defaults={"value": "a"}),
Item.update_or_create(name="test", defaults={"value": "b"}),
return_exceptions=True
)
print(results)
asyncio.run(main())The reproducer below uses the same logic as update_or_create but adds an asyncio.Barrier to force both transactions to overlap, making the race 100% reproducible:
100% reproducible test case using asyncio.Barrier
#!/usr/bin/env -S uv run --script
# /// script
# requires-python = ">=3.11"
# dependencies = ["tortoise-orm", "asyncpg"]
# ///
"""
Reproducer for update_or_create race condition in Tortoise ORM.
Requires PostgreSQL:
docker run --rm -d --name pg -e POSTGRES_PASSWORD=test -p 5432:5432 postgres:16
Run: chmod +x reproducer.py && ./reproducer.py
"""
import asyncio
from tortoise import Tortoise, fields
from tortoise.models import Model
from tortoise.transactions import in_transaction
from tortoise.exceptions import IntegrityError, DoesNotExist
DB_URL = "postgres://postgres:test@localhost:5432/postgres"
barrier = asyncio.Barrier(2)
class Item(Model):
id = fields.IntField(primary_key=True)
name = fields.CharField(max_length=100, unique=True)
value = fields.CharField(max_length=100)
async def _create_or_get(defaults, **kwargs):
"""Upstream _create_or_get logic."""
merged = {**kwargs, **defaults}
try:
async with in_transaction() as connection:
return await Item.create(using_db=connection, **merged), True
except IntegrityError as exc:
try:
return await Item.filter(**kwargs).get(), False
except DoesNotExist:
pass
raise exc
async def update_or_create_with_barrier(defaults, **kwargs):
"""Upstream update_or_create logic with barrier to force race condition."""
async with in_transaction() as connection:
instance = await Item.select_for_update().using_db(connection).get_or_none(**kwargs)
await barrier.wait() # Both transactions reach here before either continues
if instance:
await instance.update_from_dict(defaults).save(using_db=connection)
return instance, False
return await _create_or_get(defaults, **kwargs)
async def main():
await Tortoise.init(db_url=DB_URL, modules={"models": [__name__]})
await Tortoise.generate_schemas()
await Item.all().delete()
print("Running two concurrent update_or_create(name='test')...\n")
results = await asyncio.gather(
update_or_create_with_barrier({"value": "A"}, name="test"),
update_or_create_with_barrier({"value": "B"}, name="test"),
return_exceptions=True,
)
for i, r in enumerate(results, 1):
if isinstance(r, Exception):
print(f"Call {i}: {type(r).__name__}")
else:
print(f"Call {i}: created={r[1]}, value='{r[0].value}'")
await Tortoise.close_connections()
if __name__ == "__main__":
asyncio.run(main())Output:
Running two concurrent update_or_create(name='test')...
Call 1: created=False, value='B'
Call 2: created=True, value='B'
Both calls returned the same value. One caller's defaults were silently ignored. If the losing caller had created the row, the values would differ. Instead, created=False confirms it just fetched the winner's row without applying its own defaults.
Expected behavior
- Both callers should either create or update.
- The losing caller's
defaultsshould be applied viaUPDATE, not discarded. - The returned value should always reflect the provided
defaultsfor a call.
Additional context
The retry in _create_or_get masks the race by catching IntegrityError and falling back to GET, but this means:
- The losing caller's
defaultsare silently dropped - Extra round-trips (failed INSERT + GET)
I haven't found another issue which perfectly describes this, but it seems tangentially related to #1530.
Suggested fix
Use INSERT ... ON CONFLICT DO UPDATE for true atomic upsert, similar to how bulk_create already handles its on_conflict/update_fields parameters. This would require lookup fields to have a unique constraint, which is arguably correct semantics here to avoid ambiguity anyway.