`RecursionError` when deleting a model instance in Admin

[carstenfuchs]

Dear forum,

please consider these models:

from django.db import models


class Status(models.Model):
    statustext = models.CharField(max_length=20, blank=True)


class Erfasst(models.Model):
    status      = models.ForeignKey(Status, models.PROTECT)
    approved_at = models.DateTimeField(null=True, blank=True)

    def __init__(self, *args, **kwargs):
        super(Erfasst, self).__init__(*args, **kwargs)
        # Record the original values in order to be able to tell
        # later, when the object is saved back to the database,
        # if the values of the fields have changed.
        self._old_status_id   = self.status_id
        self._old_approved_at = self.approved_at

In the management shell:

$ ./manage.py makemigrations
$ ./manage.py migrate
$ ./manage.py shell

>>> from MaxRec.models import Erfasst, Status
>>> st = Status.objects.create(statustext="Hello")
>>> e = Erfasst.objects.create(status=st)

# Here comes the problem:
>>> Erfasst.from_db('default', ['id'], (1,))

# ... (long stack trace) ...

RecursionError: maximum recursion depth exceeded in comparison

Why does this happen and what can be done about it?

I originally experienced this problem when trying to delete objects via Admin action in the Admin change list view, but could reduce it to the above sample code that reproduces it with sqlite (I normally use MySQL).

[marcorichetta]

Hi!

I assume you’ve read the docs but just in case, Django doesn’t recommend to override a model’s __init__ method, instead create a classmethod for the model or a custom manager method.

If you need or want to override it, the long stack trace may help us understand the problem.

[carstenfuchs]

Hello,
thank you very much for your reply!

@marcorichetta

I assume you’ve read the docs but just in case, Django doesn’t recommend to override a model’s __init__ method, instead create a classmethod for the model or a custom manager method.

Yes, I’ve read the docs. You’re probably referring to this:

You may be tempted to customize the model by overriding the __init__ method. If you do so, however, take care not to change the calling signature as any change may prevent the model instance from being saved.

but that doesn’t seem to contradict what the __init__() method in my example code does?

Here is the beginning of the stack trace (using Django 3.2.2):

>>> Erfasst.from_db('default', ['id'], (1,))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 515, in from_db
    new = cls(*values)
  File "/home/carsten/test_max_rec_depth/MaxRec/models.py", line 17, in __init__
    self._old_status_id   = self.status_id
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query_utils.py", line 144, in __get__
    instance.refresh_from_db(fields=[field_name])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 637, in refresh_from_db
    db_instance = db_instance_qs.get()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 431, in get
    num = len(clone)
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 262, in __len__
    self._fetch_all()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 1324, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 69, in __iter__
    obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 515, in from_db
    new = cls(*values)
  File "/home/carsten/test_max_rec_depth/MaxRec/models.py", line 18, in __init__
    self._old_approved_at = self.approved_at
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query_utils.py", line 144, in __get__
    instance.refresh_from_db(fields=[field_name])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 637, in refresh_from_db
    db_instance = db_instance_qs.get()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 431, in get
    num = len(clone)
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 262, in __len__
    self._fetch_all()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 1324, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 69, in __iter__
    obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 515, in from_db
    new = cls(*values)
  File "/home/carsten/test_max_rec_depth/MaxRec/models.py", line 17, in __init__
    self._old_status_id   = self.status_id
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query_utils.py", line 144, in __get__
    instance.refresh_from_db(fields=[field_name])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 637, in refresh_from_db
    db_instance = db_instance_qs.get()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 431, in get
    num = len(clone)
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 262, in __len__
    self._fetch_all()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 1324, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 69, in __iter__
    obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 515, in from_db
    new = cls(*values)
  File "/home/carsten/test_max_rec_depth/MaxRec/models.py", line 18, in __init__
    self._old_approved_at = self.approved_at
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query_utils.py", line 144, in __get__
    instance.refresh_from_db(fields=[field_name])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 637, in refresh_from_db
    db_instance = db_instance_qs.get()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 431, in get
    num = len(clone)
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 262, in __len__
    self._fetch_all()
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 1324, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/query.py", line 69, in __iter__
    obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end])
  File "/home/carsten/.virtualenvs/Zeiterfassung/lib/python3.8/site-packages/django/db/models/base.py", line 515, in from_db
    new = cls(*values)
  File "/home/carsten/test_max_rec_depth/MaxRec/models.py", line 17, in __init__

# …

[KenWhitesell]

I did a little playing around with the code - it has something to do with having a deferred field being implicitly created by not supplying values in the from_db call.

If you comment out either of the two assignment statements, the problem goes away.

If you supply explicit values for status and approved_at in the from_db call, the problem goes away. (Supplying just one of the two keeps the problem.)

Something in the resolution of a deferred field is causing the constructor to be invoked again, causing the recursion error.

Not sure this provides any real information of value, other than possibly as an avenue for further research.

[carstenfuchs]

Hello Ken,
thanks for your reply!

Yes, I got to the same conclusions. Unfortunately, I also see this when the Django Admin app is the caller, so there is no easy work-around by one of the options that you described.

If I understand the Django code correctly, this happens because not all deferred fields are loaded at once, but only one at a time: This is why the stack trace in each recursive iteration alternates between the two fields.

And as the QuerySet get() method is used in the implementation of resolving deferred fields, which instantiates a new object, thus calling __init__(), the recursion is started in the first place.

Alas, I don’t know how to proceed from here?

[KenWhitesell]

Actually, what I think you want to do is override the from_db method in your model class - not call it from __init__. Taking a guess based upon the docs, and having absolutely no idea whether or not I’m anywhere close to being right:

@classmethod
def from_db(cls, db, field_names, values):
    instance = super().from_db(db, field_names, values)
    instance._old_status_id = instance.status_id
    instance._old_approved_at = instance.approved_at
    return instance

(Take particular note of the example and the comment line after the example.)

[carstenfuchs]

Hi Ken,

thanks for your reply and sorry for my delay!

from_db() seems to be the right place for storing loaded data, but unfortunately, the code still enters an infinite recursion. For completeness, the models.py file:

from django.db import models


class Status(models.Model):
    statustext = models.CharField(max_length=20, blank=True)


class Erfasst(models.Model):
    status      = models.ForeignKey(Status, models.PROTECT)
    approved_at = models.DateTimeField(null=True, blank=True)

    @classmethod
    def from_db(cls, db, field_names, values):
        instance = super().from_db(db, field_names, values)
        instance._old_status_id = instance.status_id
        instance._old_approved_at = instance.approved_at
        return instance

Tested at the management shell as before:

from MaxRec.models import Erfasst, Status
st = Status.objects.create(statustext="Hello")
e = Erfasst.objects.create(status=st)
Erfasst.from_db('default', ['id'], (1,))
# ends with RecursionError

The problem can be solved though by changing the from_db() implementation to only take fields into account that are in field_names, just as shown with the _loaded_values example in the docs that you linked.

However, it still seems as if the code that resolves deferred field should not be recursive by nature. If deferred fields were resolved in a straightforward manner, the problem would not exist in the first place. (I am unfortunately lacking the expertise to come up with an implementation and I’m also aware that the problem is likely not trivial.)

[KenWhitesell]

Right - you shouldn’t be calling from_db directly. You would load the rows using the standard managers methods, such as Erfasst.objects.filter(id=1)

[carstenfuchs]

Wow, you were quick!
FYI, I just edited my previous post a bit.

The problem is that it is not my custom code that is calling, but the Django Admin.

In summary, I can come up with an implementation of from_db() that works. But it seems as if the root of the problem is that the Django code resolves deferred fields both one-by-one and using a recursive approach, the combination of which causes the problem.
If fields were resolved all at once or non-recursively, the problem would not exist.

[KenWhitesell]

(You caught me on my lunch break. )

I’m not understanding how using the admin is causing a problem. I’ve got your classes running in the admin with no problem, and the admin isn’t throwing any errors. (For clarification, I can perform all four basic options in the admin on this class, create, read, update, delete.)

[carstenfuchs]

@KenWhitesell

I’m not understanding how using the admin is causing a problem. I’ve got your classes running in the admin with no problem, and the admin isn’t throwing any errors.

The problem occurs when class Erfasst has another foreign key to another class, e.g. Bereich (“department”):

class Erfasst(models.Model):
    # ...

    bereich = models.ForeignKey(Bereich, models.SET_NULL, null=True, blank=True)

If a Bereich is deleted via the Admin, the chaining nature of the deletion calls Erfasst.from_db(), thus triggering the recursion error and thus the HTTP 500 error for the client.

[KenWhitesell]

I’m still not able to recreate the problem you’re describing. Here are the classes I’m using:

class PStatus(models.Model):
    statustext = models.CharField(max_length=20, blank=True)

class Department(models.Model):
    name = models.CharField(max_length=50)

class Erfasst(models.Model):
    status      = models.ForeignKey(PStatus, models.PROTECT)
    approved_at = models.DateTimeField(null=True, blank=True)
    dept = models.ForeignKey(Department, models.SET_NULL, null=True, blank=True)

    @classmethod
    def from_db(cls, db, field_names, values):
        instance = super().from_db(db, field_names, values)
        instance._old_status_id = instance.status_id
        instance._old_approved_at = instance.approved_at
        return instance

Deleting an entry from Department that is being referenced by Erfasst causes the dept field to be set to null as expected.

[carstenfuchs]

To bring this to a (preliminary) conclusion:

The use of from_db solved my immediate problem (which was infinite recursion on deletion of a Bereich (a Department in Ken’s example code above)).

I spent a lot of time to create a testcase that demonstrates the follow-up problem that I mentioned above (still an infinite recursion when called from the Admin), but I’ve not been able to. Will keep working on it and report again if successful.

Ken, again many thanks for your help!

[mikehelmick]

Yay for searching and finding this! I’ve recently ran into this recursion error and hope to shed more light on the issue.

It looks like on the second field lookup in __init__, refresh_from_db is calling from_db recursively because:

1. “first_name” is initially deferred and Django tries to get that field
2. Model __init__ happens and tries to access “has_siblings” field but it has been deferred from [1] calling refresh_from_db with every field being deferred (from[1] the first refresh_from_db when it tries to get “first_name”)
3. Model __init__ tries to access “suffix” field and then [2] and [3] repeat, continuously trying to grab just that field, deferring every other field on the next attempt to access a field

models.py

class Person(models.Model):
    first_name = models.CharField(max_length=10)
    last_name = models.CharField(max_length=10)
    age = models.PositiveIntegerField()
    has_siblings = models.BooleanField(default=False)
    suffix = models.CharField(max_length=5)
​
    def __init__(self, *args, **kwargs):
        super(Person, self).__init__(*args, **kwargs)
        self._has_siblings = self.has_siblings
        self._suffix = self.suffix
​
    def to_dict(self):
        return {
            "first_name": self.first_name,
            "last_name": self.last_name,
            "age": self.age,
        }

Person.objects.create(first_name="Joe", last_name="S", age=30, suffic="Jr.")
Person.objects.create(first_name="Jack", last_name="Z", age=30, suffix="Sr.") ​

p = Person.objects.first()
p.to_dict()

# Causes Recursion error
p2 = Person.objects.defer("first_name").last()
p2.to_dict()

Queries

(0.001) SELECT `membership_person`.`id`, `membership_person`.`first_name` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`has_siblings` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`suffix` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`has_siblings` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`suffix` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`has_siblings` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`suffix` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`has_siblings` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.002) SELECT `membership_person`.`id`, `membership_person`.`suffix` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`has_siblings` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`suffix` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`has_siblings` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)
(0.001) SELECT `membership_person`.`id`, `membership_person`.`suffix` FROM `membership_person` WHERE `membership_person`.`id` = 2; args=(2,)

I know grabbing a field one at a time is intended, but I’m wondering if it is not intended to override the fields being deferred in the next call to the next field? i.e. In my scenario, should “has_siblings” make one call and store “has_siblings” and then not update deferred fields for the call to the next field it’s trying to get in __init__ “suffix”

Also, sorry for changing/bringing in my example to this but I didn’t want to make a new thread and it was slightly different than OPs use-case of calling from_db straight

[KenWhitesell]

But your mistake here is overriding the __init__ method. This really isn’t something you should be doing in your models as explained earlier in this thread.

[mikehelmick]

I don’t disagree. This was legacy code and is being refactored to not override init but I’m still just curious if there is a bug in how deferred fields are retrieved when somebody does have more than one fields referenced in the init or if it’s okay to chalk it up as “don’t do this, this will cause a recursion error”

[KenWhitesell]

The latter.

Keep in mind that models.Model is a heavily-metaclass driven class returning an instance of the “real” model class. (It’s responsible for handling the Meta data among many other things.) I’ve always considered it the wisest choice to try to not interfere with that, and to find the appropriate API when I need to do something out of the ordinary.

[petrklus]

Sorry for resurrecting an older thread - we are facing similar issues in Django-Filer, especifically in Django 3.2 (and potentially later), we do not seem to be able to delete users that have assets.

We have tried switching to from_db from __init__, but no luck, Django Admin still does loops ad infinitum on user deletion.

Did you have any luck resolving the Django Admin side?

[KenWhitesell]

If you’re using a custom User class (implied since you state that you’ve created a from_db method for it), it would be helpful if you posted the complete class here.

@carstenfuchs

I spent a lot of time to create a testcase that demonstrates the follow-up problem that I mentioned above (still an infinite recursion when called from the Admin), but I’ve not been able to.

I would interpret from this comment that his problem was resolved by moving away from the __init__ implementation.

I know that I have never encountered a problem when using from_db in the Django Admin.

It is likely that there’s an issue in django-filer. I started looking at their code and I see where they are overriding __init__ in their models.

[VLN37]

Feels like you guys weren’t able to get a repro on this. So i’m ressurrecting it with, hopefully, relevant info.

A very similar traceback showed at work as a result of deleting a model through the admin.

After removing the __init__ from the hypothetical Bar class below, our production problem was solved.

Prior to the solution, i tried setting these backup values pre super init, post super init and in an override of from_db, all to the same effect — infinite recursion

class Bar(models.Model):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.orig_attr = self.attr


class Foo(Bar):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.original_bar_id= self.bar_id

class Baz(Bar):
   foo = ForeignKey(Foo)

Baz.objects.first().delete()

Assumption:

Changing attrs in the init of two related models invalidates each other’s queryset cache perpetually. It may be related to the two related models inheriting from the same base.