r14389 breaks getattr on Manager subclasses

[Ian Clelland]

I've been using Simon Willison's ​QuerySetManager pattern for a while now, and since upgrading to Django 1.2.4, it has been breaking when I try to call a method on a RelatedManager constructed from it.

There was a change in r14389 (r14390 in the 1.2.X branch) which causes this code to break -- where there was a simple delegation before from the RelatedManager to the QuerySetManager to the model, there now appears to be an infinite recursion, with the RelatedManager and QuerySetManager trying to call each other's get_query_set methods.

QuerySetManager is just a simple subclass of django.db.models.Manager, which overrides __getattr__ to proxy custom method calls on the manager to a class attached to the underlying model.

The simplest test case I can get to reproduce this problem looks like this:

from django.db import models
from django.contrib.auth.models import User
from helpers import QuerySetManager

class TestModel(models.Model):
   user = models.ForeignKey(User)
   string = models.CharField(max_length=64, null=True, blank=True)
   objects = QuerySetManager()
   class QuerySet(models.query.QuerySet):
       pass

Using it gives a traceback like this:

>>> user = User.objects.get(username='testuser')
>>> user.testmodel_set.create(string="test")
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
   user.testmodel_set.create(string="test")
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 423, in create
   return super(RelatedManager, self.db_manager(db)).create(**kwargs)
 File "/Users/ian/.virtualenvs/egather/lib/python2.6/site-packages/django/db/models/manager.py", line 138, in create
   return self.get_query_set().create(**kwargs)
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
   return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
 File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
   return getattr(self.get_query_set(), attr, *args)
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
   return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
 File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
   return getattr(self.get_query_set(), attr, *args)
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
   return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
 File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
   return getattr(self.get_query_set(), attr, *args)
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
   return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
 File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
   return getattr(self.get_query_set(), attr, *args)
...
Lots snipped
...
 File "/Users/ian/.virtualenvs/egather/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
   return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
 File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 14, in get_query_set
   return self.model.QuerySet(self.model)
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/query.py", line 33, in __init__
   self.query = query or sql.Query(self.model)
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/sql/query.py", line 138, in __init__
   self.aggregates = SortedDict() # Maps alias -> SQL aggregate function
 File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/utils/datastructures.py", line 97, in __init__
   super(SortedDict, self).__init__(data)
RuntimeError: maximum recursion depth exceeded while calling a Python object

[Luke Plant]

I have followed this as exactly as I could, with a new project, and cannot reproduce it. For reference, I put the snippet directly in a single models.py, like this, just in case it makes a difference:

from django.db import models
from django.contrib.auth.models import User

class QuerySetManager(models.Manager):
    def get_query_set(self):
        return self.model.QuerySet(self.model)

class TestModel(models.Model):
   user = models.ForeignKey(User)
   string = models.CharField(max_length=64, null=True, blank=True)
   objects = QuerySetManager()
   class QuerySet(models.query.QuerySet):
       pass

I tried the example code both using "./manage.py shell", and using DJANGO_SETTINGS_MODULE and normal path stuff (I thought this might be relevant, given the points below) and both ways worked fine.

I notice 2 very strange things in your stacktrace:

1) You have django/db/models/fields/related.py from two different places - /Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/ and /Users/ian/.virtualenvs/egather/lib/python2.6/site-packages/

2) The maximum recursion depth exception appears to come after a section of stracktrace which doesn't seem to be exhibiting the mutual recursion that is displayed earlier.

It seems like something very funny is going on. So I'm going to close WORKSFORME - please re-open if you can provide more details that would allow us to reproduce this.

[Ian Clelland]

Regarding (1), that was a mistake; egather was the name of the project I was working on at the time, and I edited the stacktrace to remove the name (not as carefully as I could have, apparently, though.)

I've been able to reproduce it again, with a fresh virtualenv, and a checkout of Django 1.2.X at revision r14390, using Python 2.6.6 from MacPorts and MySQL 5.1.33.

I will attach my settings.py and models.py to this ticket.

I have dropped databases, deleted virtualenvs, and started from scratch to reproduce this. This is my project setup:

ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ mkvirtualenv testcode
New python executable in testcode/bin/python
Installing setuptools............done.
virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/predeactivate
virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/postdeactivate
virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/preactivate
virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/postactivate
virtualenvwrapper.user_scripts Creating /Users/ian/.virtualenvs/testcode/bin/get_env_details
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ ln -s ~/Code/Frameworks/django-1.2.X/django ~/.virtualenvs/testcode/lib/python2.6/site-packages/
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ ~/Code/Frameworks/django-1.2.X/django/bin/django-admin.py startproject related_test
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests$ cd related_test
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ mysql -u django -p mysql
Enter password: 
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 1711
Server version: 5.1.53 Source distribution

Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
This software comes with ABSOLUTELY NO WARRANTY. This is free software,
and you are welcome to modify and redistribute it under the GPL v2 license

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> create database related_test;
Query OK, 1 row affected (0.00 sec)

mysql> \q
Bye
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ ./manage.py startapp test_app
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$

I then edited settings.py and test_app.models.py -- see attached files

Creating a user object, and then trying to create a related TestModel instance is sufficient to see this behaviour:

(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ ./manage.py syncdb
Creating table auth_permission
Creating table auth_group_permissions
Creating table auth_group
Creating table auth_user_user_permissions
Creating table auth_user_groups
Creating table auth_user
Creating table auth_message
Creating table django_content_type
Creating table test_app_testmodel

You just installed Django's auth system, which means you don't have any superusers defined.
Would you like to create one now? (yes/no): no
Installing index for auth.Permission model
Installing index for auth.Group_permissions model
Installing index for auth.User_user_permissions model
Installing index for auth.User_groups model
Installing index for auth.Message model
Installing index for test_app.TestModel model
No fixtures found.
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ ./manage.py shell
Python 2.6.6 (r266:84292, Dec 29 2010, 09:13:15) 
[GCC 4.2.1 (Apple Inc. build 5664)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django.contrib.auth.models import User
>>> user = User.objects.create_user('testuser', 'test@example.com')
>>> user.id
1L
>>> user.testmodel_set.create(string='test')
Exception RuntimeError: 'maximum recursion depth exceeded in __subclasscheck__' in <type 'exceptions.AttributeError'> ignored
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 423, in create
    return super(RelatedManager, self.db_manager(db)).create(**kwargs)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/manager.py", line 138, in create
    return self.get_query_set().create(**kwargs)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
    return getattr(self.get_query_set(), attr, *args)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
    return getattr(self.get_query_set(), attr, *args)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
    return getattr(self.get_query_set(), attr, *args)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
    return getattr(self.get_query_set(), attr, *args)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
    return getattr(self.get_query_set(), attr, *args)

...

  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
    return getattr(self.get_query_set(), attr, *args)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 17, in __getattr__
    return getattr(self.get_query_set(), attr, *args)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/fields/related.py", line 410, in get_query_set
    return superclass.get_query_set(self).using(db).filter(**(self.core_filters))
  File "/Users/ian/Code/Tests/related_test/test_app/models.py", line 14, in get_query_set
    return self.model.QuerySet(self.model)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/query.py", line 33, in __init__
    self.query = query or sql.Query(self.model)
  File "/Users/ian/.virtualenvs/testcode/lib/python2.6/site-packages/django/db/models/sql/query.py", line 127, in __init__
    self.where = where()
RuntimeError: maximum recursion depth exceeded while calling a Python object
>>>

My django checkout is clean:

(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Tests/related_test$ cd ~/Code/Frameworks/django-1.2.X
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-1.2.X$ svn info

Path: .
URL: http://code.djangoproject.com/svn/django/branches/releases/1.2.X
Repository Root: http://code.djangoproject.com/svn
Repository UUID: bcc190cf-cafb-0310-a4f2-bffc1f526a37
Revision: 14390
Node Kind: directory
Schedule: normal
Last Changed Author: russellm
Last Changed Rev: 14390
Last Changed Date: 2010-10-28 06:00:08 -0700 (Thu, 28 Oct 2010)

(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-1.2.X$ svn diff
(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-1.2.X$

If I revert to revision r14388, this behaviour doesn't occur. (r14389 is the same changeset as r14390, applied to trunk rather than 1.2.X)

I have also tested this on django-trunk:

(testcode)ian@Ian-Clellands-MacBook-Pro:~/Code/Frameworks/django-trunk$ svn info
Path: .
URL: http://code.djangoproject.com/svn/django/trunk
Repository Root: http://code.djangoproject.com/svn
Repository UUID: bcc190cf-cafb-0310-a4f2-bffc1f526a37
Revision: 15195
Node Kind: directory
Schedule: normal
Last Changed Author: russellm
Last Changed Rev: 15194
Last Changed Date: 2011-01-13 08:41:46 -0800 (Thu, 13 Jan 2011)

With the same results; except that the relevant line in related.py is 412, rather than 410.

Please let me know if there is anything else I can provide to help reproduce this.

[Ian Clelland]

settings.py for test project which reproduces error

[Ian Clelland]

test_app/models.py for test project which reproduces error

[Ian Clelland]

Interestingly, I didn't even realise that I wasn't using QuerySetManager straight from the snippet (the comment referencing djangosnippets is in my code, but I've been using it for so long that I forgot where my current version *actually* came from).

The comments at the bottom of ​http://djangosnippets.org/snippets/734/ show the full version I am using (specifically, the addition in ​http://djangosnippets.org/snippets/734/#c903). It is described in more detail at ​http://seanmonstar.com/post/708862164/extending-django-models-managers-and-querysets. The models.py file I attached also includes it.

class QuerySetManager(models.Manager):
	def get_query_set(self):
		return self.model.QuerySet(self.model)
	def __getattr__(self, attr, *args):
		return getattr(self.get_query_set(), attr, *args)

The important (and breaking) bit is the __getattr__ override -- that used to delegate (through the RelatedManager parent) to the queryset defined in the model; now, it looks to me like the RelatedManager is calling get_query_set again, which somehow gets back to the QuerySetManager code, and starts the cycle anew, and so on until the stack limit is exceeded.

[Karen Tracey]

Marking as regression since what worked in 1.2.3 is breaking in 1.2.4. I don't know if a fix is possible, but if not this might warrant at least a note in the release notes...I know of one project that had to back off 1.2.4 due to running into this (plus another regression that has been fixed).

[Łukasz Rekucki]

From what I managed to debug, the recursion starts in call to self.db_manager(db) which tries to copy() the manager instance.

Your custom manager intercepts calls (and hasattr checks) of __getstate__ and __setstate__ to a freshly created QuerySet instance. Python tries to use those method to copy the manager. A hasattr() check for __setstate__ is caught by __getattr__ which executes self.get_query_set(). But that assumes that the manager instance has gone through proper initialization... so a lookup for a field like self._db fails and hits the __getattr__. Boom!.

I'm in favour of marking this as invalid, as you shouldn't be passing magic methods like __getstate__ to other objects. Personally, I wouldn't pass anything marked as private, like this:

   def __getattr__(self, attr, *args):
        if attr.startswith("_"): # or at least "__"
            raise AttributeError
        return getattr(self.get_query_set(), attr, *args)

With the code above, the testcase you provided works fine.

[Ian Clelland]

I can confirm that it works with Łukasz' additions (either the single- or double-underscore version) that the manager works correctly. This works around the issue on both 1.2.X and SVN.

I don't know whether '_' or '__' is more correct in this case -- calling a special method on a manager should certainly not try to delegate to the queryset, but I don't know about attributes that are just private by convention.

[Russell Keith-Magee]

I'm happy to call this a case of some slightly naive code being caught out when put under more rigorous conditions. However, the rigorous conditions aren't especially surprising -- I don't think it's especially onerous to suggest that a Manager object should be able to be shallow copied.

The documentation should be clarified to point out this limitation, but it's not a regression that needs a code fix.

[Russell Keith-Magee]

(In [15220]) Fixed #15062 -- Documented the fact that managers must be able to be shallow copied. Thanks to Ian Clelland for the report, and Łukasz Rekucki for the help diagnosing the problem.

[Russell Keith-Magee]

(In [15221]) [1.2.X] Fixed #15062 -- Documented the fact that managers must be able to be shallow copied. Thanks to Ian Clelland for the report, and Łukasz Rekucki for the help diagnosing the problem.

Backport of r15220 from trunk.

[Jacob]

Milestone 1.3 deleted