Summary
While investigating #640, I found two independent, concretely reproducible concurrency bugs in MultiObjectFieldType.create_polymorphic_m2m_table() (called once, when a polymorphic multiobject field is first created). Neither reproduces #640's specific reported symptom (see the discussion on PR #648) -- these are separate bugs in the same function, verified with deterministic regression tests.
Bug 1: registration-before-repoint race against a concurrent reader
create_polymorphic_m2m_table() built and registered a through-model class with Django's app registry, and only afterward repointed its source FK at the caller's model class -- all without holding CustomObjectType._global_lock. CustomObjectType.get_model()'s own reuse-or-create check for polymorphic through models (_after_model_generation()) is lock-protected on its own side. A concurrent get_model(no_cache=True) call could land in the writer's build-then-repoint window, find the through model already registered, and repoint source at its own (different, but table-equivalent) model instance instead -- leaving the through's FK and whatever get_model() subsequently caches pointing at two different Python classes for the same table.
That class-identity mismatch produces ValueError: Cannot query "X": Must be "TableYModel" instance. or a RecursionError when later deleting an object through the affected relation.
Bug 2: lock-ordering deadlock on a double-submit
Fixing bug 1 by holding _global_lock across the whole create_polymorphic_m2m_table() call (including the table-existence probe and schema_editor.create_model() DDL) introduces a different real deadlock: two threads racing to create the same field (e.g. a doubly-clicked "save" button, or a retried request) each build+register a through model for the same physical table before either knows which one will win the (name, custom_object_type) UniqueConstraint. Whichever thread's schema_editor.create_model() runs second blocks at the Postgres level waiting on the first thread's uncommitted CREATE TABLE (same table name) to resolve -- but the first thread's own CustomObjectTypeField.save() needs _global_lock again in clear_model_cache() before it can commit and release that wait. Confirmed live via pg_stat_activity: one thread idle-in-transaction waiting on the lock, the other actively blocked on Lock/transactionid executing CREATE TABLE.
Steps to reproduce
Both are exercised by deterministic regression tests added in PR #648:
PolymorphicMultiObjectConcurrencyTestCase.test_forced_registration_interleaving_stays_consistent (bug 1) -- forces the exact registration-before-repoint interleaving via a mocked apps.register_model() hook.
PolymorphicMultiObjectConcurrencyTestCase.test_concurrent_double_submit_does_not_deadlock (bug 2) -- two threads calling CustomObjectTypeField.objects.create() for the identical (custom_object_type, name) at once.
Relationship to #640
#640 reports a different, more severe symptom (a deterministic FieldDoesNotExist on a fresh, single, non-concurrent process) that I was unable to reproduce on current main -- see the investigation notes on PR #648. This issue is scoped narrowly to the two concurrency bugs above, which PR #648 fixes with regression coverage. #640 remains open pending further reproduction details.
Summary
While investigating #640, I found two independent, concretely reproducible concurrency bugs in
MultiObjectFieldType.create_polymorphic_m2m_table()(called once, when a polymorphicmultiobjectfield is first created). Neither reproduces #640's specific reported symptom (see the discussion on PR #648) -- these are separate bugs in the same function, verified with deterministic regression tests.Bug 1: registration-before-repoint race against a concurrent reader
create_polymorphic_m2m_table()built and registered a through-model class with Django's app registry, and only afterward repointed itssourceFK at the caller's model class -- all without holdingCustomObjectType._global_lock.CustomObjectType.get_model()'s own reuse-or-create check for polymorphic through models (_after_model_generation()) is lock-protected on its own side. A concurrentget_model(no_cache=True)call could land in the writer's build-then-repoint window, find the through model already registered, and repointsourceat its own (different, but table-equivalent) model instance instead -- leaving the through's FK and whateverget_model()subsequently caches pointing at two different Python classes for the same table.That class-identity mismatch produces
ValueError: Cannot query "X": Must be "TableYModel" instance.or aRecursionErrorwhen later deleting an object through the affected relation.Bug 2: lock-ordering deadlock on a double-submit
Fixing bug 1 by holding
_global_lockacross the wholecreate_polymorphic_m2m_table()call (including the table-existence probe andschema_editor.create_model()DDL) introduces a different real deadlock: two threads racing to create the same field (e.g. a doubly-clicked "save" button, or a retried request) each build+register a through model for the same physical table before either knows which one will win the(name, custom_object_type)UniqueConstraint. Whichever thread'sschema_editor.create_model()runs second blocks at the Postgres level waiting on the first thread's uncommittedCREATE TABLE(same table name) to resolve -- but the first thread's ownCustomObjectTypeField.save()needs_global_lockagain inclear_model_cache()before it can commit and release that wait. Confirmed live viapg_stat_activity: one thread idle-in-transaction waiting on the lock, the other actively blocked onLock/transactionidexecutingCREATE TABLE.Steps to reproduce
Both are exercised by deterministic regression tests added in PR #648:
PolymorphicMultiObjectConcurrencyTestCase.test_forced_registration_interleaving_stays_consistent(bug 1) -- forces the exact registration-before-repoint interleaving via a mockedapps.register_model()hook.PolymorphicMultiObjectConcurrencyTestCase.test_concurrent_double_submit_does_not_deadlock(bug 2) -- two threads callingCustomObjectTypeField.objects.create()for the identical(custom_object_type, name)at once.Relationship to #640
#640 reports a different, more severe symptom (a deterministic
FieldDoesNotExiston a fresh, single, non-concurrent process) that I was unable to reproduce on currentmain-- see the investigation notes on PR #648. This issue is scoped narrowly to the two concurrency bugs above, which PR #648 fixes with regression coverage. #640 remains open pending further reproduction details.