Opened 14 years ago

Closed 14 years ago

Last modified 4 years ago

#13227 closed (fixed)

Query cloning fails with models having Lock as instance member

Reported by: Claude Paroz Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here is a basic test case:

import threading
from django.db import models

class BaseObject(models.Model):
    status      = models.CharField(max_length=10)
    foreign     = models.ForeignKey('MyModel')

class MyModel(models.Model):
    name = models.CharField(max_length=10)

    def __init__(self, *args, **kwargs):
        super(MyModel, self).__init__(*args, **kwargs)
        self.checkout_lock = threading.Lock()

Now as soon as a query with MyModel instance has to be cloned, this fails.

from testapp.models import MyModel, BaseObject
mm = MyModel(name='Test1')
mm.save()
bo = BaseObject(status='a',foreign=mm)
bo.save()

print BaseObject.objects.filter(foreign=mm).order_by('foreign__name')
(full traceback here)
Error: un(deep)copyable object of type <type 'thread.lock'>

Change History (12)

comment:1 by Claude Paroz, 14 years ago

Also note that this doesn't happen with 1.1

comment:2 by George Vilches, 14 years ago

milestone: 1.2

If this doesn't happen in 1.1, but does in 1.2, the should this be marked as a regression for 1.2 release? Marking the milestone so a core dev will notice and decide, since that fits the criteria from other tickets.

comment:3 by Alex Gaynor, 14 years ago

Triage Stage: UnreviewedAccepted
Version: 1.2-betaSVN

comment:4 by Russell Keith-Magee, 14 years ago

I'm looking at this right now; it's a problem with the way db_prep_save is used on foreign keys. Should have a patch soon.

comment:5 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: newclosed

(In [12865]) Fixed #13227 -- Modified ForeignKeys to fully honor the db_prep/prep separation introduced by multidb. This was required to ensure that model instances aren't deepcopied as a result of being involved in a filter clause. Thanks to claudep for the report, and Alex Gaynor for the help on the patch.

comment:6 by Claude Paroz, 14 years ago

Resolution: fixed
Status: closedreopened

I'm so sorry, but I think this is not completely resolved. Here is an updated test case where the same error appear. Model is unchanged.

from testapp.models import MyModel, BaseObject
mm = MyModel(name='Test1')
mm.save()
bo = BaseObject(status='a',foreign=mm)
bo.save()

mms = MyModel.objects.all()
mms_id_list = [m.id for m in mms]
print BaseObject.objects.filter(foreign__in=mms).order_by('foreign__name')

Note the mms_id_list creation (apparently unrelated to the problem) which access the id of MyModel objects. Without this line, the query is successfull.

comment:7 by Russell Keith-Magee, 14 years ago

The issue is that by evaluating mms_id_list (which evaluates the mms queryset), you populate the mms queryset cache with object instances; when you then try to evaluate the BaseObject query, you need to be able to clone those instances.

On closer inspection, it looks like the original problem was also present in 1.1... but this problem *isn't*, so it's a regression. However I also need to backport r12865 to 1.1.

comment:8 by Russell Keith-Magee, 14 years ago

(In [12963]) [1.1.X] Refs #13227 -- Partial backport of r12865; backported the changes to Where tree cloning logic to ensure that unclonable objects in a where() clause don't break querying.

comment:9 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: reopenedclosed

(In [12970]) Fixed #13227 -- Ensure that the query cache is flushed when a QuerySet is deepcopied, avoiding problems when an evaluated queryset is used as a subquery. Thanks to claudep for the report.

comment:10 by Claude Paroz, 14 years ago

Thanks Russell, it's fine now!

comment:11 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Easy pickings: unset
UI/UX: unset

In 1dd96f73:

Refs #13227 -- Adjusted a test to avoid making a shared test model unpickable.

This allowed the Note model to be used in setUpTestData() which requires
assigned model instances to be copy.deepcopy()'able.

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