OneToOneField + concurrent get_or_create results in wrong object in field cache

[Brian Atkinson]

When using get_or_create and update_or_create in a context where concurrent creations are happening, the caller that loses the creation race will end up with an incomplete object in the field cache.

Example models:

from django.db import models

class Parent(models.Model):
    name = models.TextField()

class Child(models.Model):
    parent = models.OneToOneField(Parent, on_delete=models.CASCADE)

In effect, we have two separate processes both expecting to be able to call Child.objects.get_or_create(parent=common_parent, defaults={...}) and have the resulting common_parent.child be the correct result. We are seeing both processes fail the initial get call (​https://github.com/django/django/blob/5.2.3/django/db/models/query.py#L946) and proceeding to attempt the create call (​https://github.com/django/django/blob/5.2.3/django/db/models/query.py#L953). One of the processes succeeds on the insert, and the other will fail the insert. The process that fails the insert correctly issues the second get call (​https://github.com/django/django/blob/5.2.3/django/db/models/query.py#L956) and returns the result.

The bug is that for the process that fails the insert, the object at common_parent.child is not the value returned from the get_or_create. It seems to be the object that was created for insert, but wasn't cleared when the insert failed. This can be observed through the common_parent.child.id field being None. This has caused data corruption and other downstream bugs due to the wrong data being cached.

Here is a test case that is a linear representation of the concurrent bug described above:

from unittest import mock

from django.db.models import QuerySet
from django.test import TestCase

from sample.models import Child, Parent

class TestBug(TestCase):
    def test_bug(self):
        parent = Parent.objects.create(name="Test name")

        triggered = False

        def effect(*args, **kwargs):
            nonlocal triggered
            if not triggered and kwargs.get("parent") == parent:
                triggered = True
                Child.objects.update_or_create(
                    # Force a brand new instance (emulates racing thread).
                    parent=Parent.objects.get(pk=parent.pk),
                    defaults={},
                )
            return mock.DEFAULT

        with mock.patch.object(
            QuerySet,
            "_extract_model_params",
            autospec=True,
            wraps=QuerySet._extract_model_params,
            side_effect=effect,
        ):
            child, created = Child.objects.get_or_create(parent=parent, defaults={})
            assert not created

        assert child.parent_id is not None
        assert parent.child.id is not None

At first glance this test case will seem quite janky. The patching is being done to emulate the effect of two racing threads by injecting a concurrent call to get_or_create in the middle of the outer get_or_create.