Django View classes and thread safety of mutable class attributes

Written by Johannes on December 4, 2012 Categories: Django

I found this intriguing bug (or is it a feature? it is somewhat documented in the docs) in the implementation of Djangos View classes.

I wrote the following little class (somewhat simplified):

class TherapyView(TemplateView):
    template_name = "therapy.html"
    medication_form = MedicationForm
 
    medication_initial = []
 
    def prepare_data(self):
        qs = Therapy.objects.get(pk=self.kwargs.get('pk')).medication_set.all()
        for med in qs:
            self.medication_initial.append({
                'substance' : med.substance.title,
                'dosage' : med.dosage,
            })
 
    def get_medication_formset(self):
        """ return an instance of the medication formset """
        from django.forms.formsets import formset_factory
        return formset_factory(
                    self.medication_form,
                    formset=MedicationBaseFormSet
                )(initial=self.medication_initial)
 
    def get(self, request, *args, **kwargs):
        """ handle get request, return HttpResponse object on success """
        context['formset'] = self.get_medication_formset()
        return self.render_to_response(context)

The strange side effect of this was that on every call to the view, medication_initial was still filled with data from the former call. Subsequent calls to the view stacked the formset with more and more entries. This obviously could lead to some unwanted results and I would consider this to be a bug in Django.

But on reading the docs I found out that similar behavior is documented for class attributes that are initialized by View.as_view(). Here the docs warn against passing lists as arguments to as_view() as this isn’t thread safe (see https://docs.djangoproject.com/en/dev/ref/class-based-views/#specification). Apparently references to the class attribute are shared between instances of the View class.

~~My temporary workaround for~~ The solution to this issue (note the lower discussion: this behavior is inherent in the way Python works) is to initialize the list to None and to overwrite it with a list created in local scope:

class TherapyView(TemplateView):
    # [...]
    medication_initial = None
 
    def prepare_data(self):
        qs = Therapy.objects.get(pk=self.kwargs.get('pk')).medication_set.all()
        medication_initial = []
        for med in qs:
            medication_initial.append({
                'substance' : med.substance.title,
                'dosage' : med.dosage,
            })
        self.medication_initial = medication_initial
 
    def get_medication_formset(self):
        # [...]
 
    def get(self, request, *args, **kwargs):
        # [...]

I haven’t had the opportunity yet to test this for concurrent access to the same view, but for serial access this appears to be stable.

2 Comments

prinz says:

April 6, 2013 at 7:49 am

Why do you consider it to be a bug of Django? This is how Python works. Consider this:

>>> class Therapy(object):
… alist = []
… def append(self, item):
… self.alist.append(item)
…
>>> t1 = Therapy()
>>> t1.append(’3′)
>>>
>>> t2 = Therapy()
>>> t2.alist
['3']

This is because classes are treated as objects too. And alist is a class attribute that is scoped to the class’ lifecycle. If you want alist to be bound to the scope of the class instance, you need to deal with it as instance attribute. E.g.

>>> def __init__(self):
>>> self.alist = []

Cheers, prinz

Reply
Johannes says:

April 6, 2013 at 9:40 am

When I wrote the article I wasn’t quite aware of the fact that this is the way Python handles class attributes. Coming from C++ I still have that gut feeling that variables should be forward declared. Add to that the “magic-ish” way class based views are dispatched in Django.

By now I’m obviously older and wiser and better used to the way Python does things. I’m also older and wiser and try to avoid most of the fancy stuff Django provides with generic class based views and just stick to inheriting TemplateView and doing all the rest myself.

Your hint towards using the constructor to instantiate instance attributes rather than declaring them in the class scope is very valuable, thanks.

Reply