0

I encountered a bug in my Django app yesterday and even though I fixed it since, I still do not understand its cause, neither how I resolved it.

Well, actually I found the root cause while writing this question, thanks to SO "Questions with similar titles" feature. See "Least Astonishment" and the Mutable Default Argument

Buuut, I still don't understand how could that affect my app in the way it affected it. So, let's dig in.

I have a web page, which display a list of items. I query those items through the Model.get method, from the views.py file. Nothing out of the ordinary, basically fetching the DB from the model, calling the model from the view and providing a variable with the fetched values to the template.

When using the faulty source code, I would refresh the page and the items would randomly either appear, or disappear. I figured the DB query would either return the items, or return an empty list.

Here is the faulty source code (model.py):

  @classmethod
  def get(cls, school=None, additional_filters={}):
    if school:
      additional_filters['school'] = school

    return MyModel.objects.filter(
      **additional_filters
    )

And here is how I fixed it:

  @classmethod
  def get(cls, school=None, additional_filters=None):
    if not additional_filters:
      additional_filters = {}

    if school:
      additional_filters['school'] = school

    return MyModel.objects.filter(
      **additional_filters
    )

I fixed it this way because PyCharm IDE told me there was something wrong Default argument value is mutable and since I couldn't explain the bug at all, I followed its recommendations.

But I still don't understand why. And even now, after reading "Least Astonishment" and the Mutable Default Argument I still don't.

I understand now that the additional_filters were modified at every call due to the way Python handles default function arguments in memory.

What I don't explain is the side effect of this behaviour. Why did the query returned either the proper items or an empty set? Especially considering the code wasn't providing any additional_filters, meaning the only added item in additional_filters was school which was always the same at every query.

That's the part I really don't understand. All my calls to this method were of the form Model.get(request.context.school), and since additional_filters is a map and not an array, it should always have contained the same value within.

This bug took me a while to figure out, because I couldn't reproduce it on my local environment nor in the staging environment, it affected only the production environment and made it very very difficult to locate.

Vadorequest
  • 16,593
  • 24
  • 118
  • 215
  • Are you by any chance running your project in multiple processes in your production environment? Also does your staging environment differ from the production environment and if so how? – Fynn Becker Jan 17 '19 at 12:38
  • @FynnBecker The staging and production don't differ at all. They're hosted on the same server, connected to a different DB but the DB version is the same for both. Django is running in FCGI mode. I don't know if that means multiple processes though. – Vadorequest Jan 17 '19 at 13:40
  • 1
    As a general rule, consider that a production setup for a django (or any wsgi app FWIW) project WILL run in multiprocess mode - and we're talking long running processes here, not "one shot" processes à la PHP. IOW, __any__ kind of mutable global state (mutable default arguments are just one example of mutable global state) must be avoided in such applications (this also applies to any code at a module or class top-level). – bruno desthuilliers Feb 11 '19 at 12:59
  • This actually explains why I couldn't reproduce the bug neither in development nor in staging environment, thanks! – Vadorequest Feb 12 '19 at 17:01

1 Answers1

2

FCGI maintains a pool of processes. Each of the pool members has an empty dict as a default keyword argument.
You are initially lucky and the requests reach the processes with unchanged empty dict. But whenever a request hits the process which already altered this dictionary, then it's not an empty dict anymore - your filters accumulate.

ElmoVanKielmo
  • 10,907
  • 2
  • 32
  • 46