Opened 2 years ago

Closed 22 months ago

Last modified 22 months ago

#20493 closed Bug (fixed)

Add a warning that objects may not be picklable across Django versions

Reported by: cataliniacob Owned by: nobody
Component: Documentation Version: 1.5
Severity: Normal Keywords:
Cc: iacobcatalin@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I just upgraded from 1.4 to 1.5 and this took a while to track.

If one has a cache that persists across a Django version change (filesystem cache, memcached that is not restarted), when executing any view decorated with @cache_page unpickling the HttpResponse from the cache leads to strange errors. For example, when upgrading from 1.4 to 1.5 I get:

[24/May/2013 15:53:42] "GET / HTTP/1.1" 200 5
Traceback (most recent call last):
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 86, in run
    self.finish_response()
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 129, in finish_response
    self.close()
  File "/usr/lib64/python2.7/wsgiref/simple_server.py", line 36, in close
    SimpleHandler.close(self)
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 257, in close
    self.result.close()
  File "/home/catalin/hacking/envs/cached-httpresponse-across-versions/lib/python2.7/site-packages/django/http/response.py", line 231, in close
    for closable in self._closable_objects:
AttributeError: 'HttpResponse' object has no attribute '_closable_objects'
[24/May/2013 15:53:42] "GET / HTTP/1.1" 500 59
----------------------------------------
Exception happened during processing of request from ('127.0.0.1', 49852)
Traceback (most recent call last):
  File "/usr/lib64/python2.7/SocketServer.py", line 582, in process_request_thread
    self.finish_request(request, client_address)
  File "/usr/lib64/python2.7/SocketServer.py", line 323, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/home/catalin/hacking/envs/cached-httpresponse-across-versions/lib/python2.7/site-packages/django/core/servers/basehttp.py", line 150, in __init__
    super(WSGIRequestHandler, self).__init__(*args, **kwargs)
  File "/usr/lib64/python2.7/SocketServer.py", line 638, in __init__
    self.handle()
  File "/usr/lib64/python2.7/wsgiref/simple_server.py", line 124, in handle
    handler.run(self.server.get_app())
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 89, in run
    self.handle_error()
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 304, in handle_error
    self.finish_response()
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 127, in finish_response
    self.write(data)
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 210, in write
    self.send_headers()
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 267, in send_headers
    if not self.origin_server or self.client_is_modern():
  File "/usr/lib64/python2.7/wsgiref/handlers.py", line 280, in client_is_modern
    return self.environ['SERVER_PROTOCOL'].upper() != 'HTTP/0.9'
TypeError: 'NoneType' object has no attribute '__getitem__'
----------------------------------------

This was at first quite perplexing because I checked response.py and self._closable_objects gets assigned in HttpResponseBase.__init__ which gets called by HttpResponse.__init__ so it seemed impossible for an HttpResponse instance not to have the attribute. Unpickling doesn't normally call __init__ but this is not at all obvious or easy to track.

There are similar issues but with a bit more clear stacktrace if downgrading from 1.5 to 1.4:

Traceback:
File "/home/catalin/hacking/envs/cached-httpresponse-across-versions/lib/python2.7/site-packages/django/core/handlers/base.py" in get_response
  89.                     response = middleware_method(request)
File "/home/catalin/hacking/envs/cached-httpresponse-across-versions/lib/python2.7/site-packages/django/middleware/cache.py" in process_request
  147.         response = self.cache.get(cache_key, None)
File "/home/catalin/hacking/envs/cached-httpresponse-across-versions/lib/python2.7/site-packages/django/core/cache/backends/filebased.py" in get
  41.                     return pickle.load(f)

Exception Type: ImportError at /
Exception Value: No module named response

I'm surprised nobody seems to have run into this so far, googling for AttributeError on _closable_objects and searching the other bugs didn't show anything. I would imagine it's reasonably common to upgrade without restarting memcached or to have a filesystem based cache on a development machine where you don't bother to install memcached. And this happens on every view with @cache_page.

I don't know what a good solution would be. I can imagine Django doesn't want to guarantee pickle/unpickle round trips across versions as those can be hard to maintain but I think there should at least be a warning in the upgrade notes (of every version): when upgrading Django you need to delete your cache.

Attachments (2)

20493.diff (754 bytes) - added by timo 22 months ago.
20493.2.diff (874 bytes) - added by timo 22 months ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 2 years ago by cataliniacob

  • Cc iacobcatalin@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 2 years ago by carljm

  • Component changed from Core (Cache system) to Documentation
  • Triage Stage changed from Unreviewed to Accepted

I think you're right that a requirement of cross-version unpicklability is more than we want to commit to. We do have an "upgrading Django" page in the docs now (https://docs.djangoproject.com/en/1.5/howto/upgrade-version/); seems like it would make a lot of sense to add this as a note there.

On a more long-term note, it might be worth considering whether the caching middleware should cache a very simple "body, headers" data structure instead of a full pickled HttpResponse. This would probably break all kinds of code, though, since it means anything else tacked on to the HttpResponse (and perhaps used by middlewares further down the chain) would be missing.

Changed 22 months ago by timo

comment:3 Changed 22 months ago by timo

  • Has patch set
  • Summary changed from HttpResponse objects cached with cache_page don't unpickle across Django version leading to strange errors to Add a warning that objects may not be picklable across Django versions

Does the attached patch make sense? Would you say anything else?

comment:4 Changed 22 months ago by cataliniacob

I would make the suggestion a bit stronger, something like:
... you should consider clearing your cache after upgrading. Otherwise you may run into problems ...

I don't know if people clearly make the connection between "I'm using @cache_page" and "I'm caching pickled HttpResponse objects", so I see the stronger suggestion as a hint that you shouldn't just dismiss the next section (doesn't apply to me, I'm not pickling anything, I'm just using @cache_page).

But in general it sounds fine to me!

Changed 22 months ago by timo

comment:5 Changed 22 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 10845751632e843aeb21a030c7df7f9848bf65aa:

Fixed #20493 -- Added a warning that objects may not be picklable across Django versions

Thanks cataliniacob for the suggestion and review.

comment:6 Changed 22 months ago by Tim Graham <timograham@…>

In f551d5a3de4f32f0b6322a1d884990e34ab67528:

[1.5.x] Fixed #20493 -- Added a warning that objects may not be picklable across Django versions

Thanks cataliniacob for the suggestion and review.

Backport of 1084575163 from master

comment:7 Changed 22 months ago by Tim Graham <timograham@…>

In 67a6b5e53f62e90c7e8587d1f110aa3d7bb9afbf:

[1.6.x] Fixed #20493 -- Added a warning that objects may not be picklable across Django versions

Thanks cataliniacob for the suggestion and review.

Backport of 1084575163 from master

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