Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#13366 closed (fixed)

Model importing race condition created in [12977] for subclasses of abstract classes that have fields.

Reported by: bretthoerner Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords:
Cc: adulic@…, safariman@…, davisd.davisd@…, rokclimb15@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


We ran into a weird edge case race condition that seems to be caused by [12977]. FWIW I haven't taken all the time needed to understand exactly why or what [12977] does, I'm just reporting the error based on a traceback and example code set. :)

Basic things you need:

  1. An abstract base class with a field, and a model A that uses it, so that ModelBase.__new__ will drop into the "new_class.add_to_class(, copy.deepcopy(field))" branch intended for abstract only.
  2. Another model (in a different module) that imports model A.
  3. A management command that imports model A (this won't happen in the shell, I'm not sure what the difference is yet or where else it would occur).

Basically it seems that the new "self.default = self.model._meta.get_field_by_name([0].default" results eventually in a "get_models()", but this is run *while model A is being loaded* which means that all models are fetched, including the models file which contains model B and thus imports model A, and then blammo, circular dependency problem.

I've attached a simple project with two apps and a sample management command.

Attachments (2)

13366-example.tar.gz (3.0 KB) - added by bretthoerner 6 years ago.
Simple example project, run python fail
13366.symptomaticpatch (1.6 KB) - added by erik 6 years ago.
Attached symptomatic patch for using the comments app (worksforme).

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by bretthoerner

Simple example project, run python fail

comment:1 Changed 6 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

#13368 and a post in django-dev ( report a manifestation of this when trying to import django.contrib.comments.models.Comment from an app models module.

comment:2 Changed 6 years ago by russellm

  • milestone set to 1.2
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 6 years ago by russellm

  • Component changed from Uncategorized to Core framework

comment:4 Changed 6 years ago by russellm

I think the solution here is to force the population of the application cache before management commands are used. The simple approach of putting "get_apps()" early in the management command execute cycle fixes this particular problem, but it causes other regressions in the test suite (mostly dealing with error handling of applications that can't be imported, etc).

comment:5 Changed 6 years ago by anonymous

  • Cc adulic@… added

comment:6 Changed 6 years ago by anonymous

  • Cc safariman@… added

comment:7 Changed 6 years ago by patrys

It would be better to guard the self.model access with a hasattr check. copy() should be safe to call at *all* times. Postponing the call is a workaround, not a fix.

Changed 6 years ago by erik

Attached symptomatic patch for using the comments app (worksforme).

comment:8 Changed 6 years ago by davisd

  • Cc davisd.davisd@… added

comment:9 Changed 6 years ago by rokclimb15

  • Cc rokclimb15@… added

comment:10 Changed 6 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [13005]) Fixed #13366 -- Corrected the field setstate method to avoid a race condition when initially importing models. Thanks to Brett Hoerner for the report.

comment:11 Changed 5 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top