Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#20493 closed Bug (fixed)

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

Reported by: Catalin Iacob 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 Tim Graham 11 years ago.
20493.2.diff (874 bytes ) - added by Tim Graham 11 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by Catalin Iacob, 11 years ago

Cc: iacobcatalin@… added

comment:2 by Carl Meyer, 11 years ago

Component: Core (Cache system)Documentation
Triage Stage: UnreviewedAccepted

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.

by Tim Graham, 11 years ago

Attachment: 20493.diff added

comment:3 by Tim Graham, 11 years ago

Has patch: set
Summary: HttpResponse objects cached with cache_page don't unpickle across Django version leading to strange errorsAdd a warning that objects may not be picklable across Django versions

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

comment:4 by Catalin Iacob, 11 years ago

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!

by Tim Graham, 11 years ago

Attachment: 20493.2.diff added

comment:5 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: newclosed

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 by Tim Graham <timograham@…>, 11 years ago

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 by Tim Graham <timograham@…>, 11 years ago

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