#16241 closed Bug (fixed)
Django runserver (<=1.3) is not WSGI compliant.
Reported by: | Graham Dumpleton | Owned by: | Aymeric Augustin |
---|---|---|---|
Component: | Core (Other) | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The Django runserver doesn't call close() on the iterable returned from WSGI application in all circumstances which is a violation of WSGI specification.
The specific scenario where close() is not called is where a socket error occurs writing response content after headers have been written. For example for:
Traceback (most recent call last): File "/Library/WebServer/Sites/django-4/lib/python2.6/site-packages/Django-1.3-py2.6.egg/django/core/servers/basehttp.py", line 284, in run self.finish_response() File "/Library/WebServer/Sites/django-4/lib/python2.6/site-packages/Django-1.3-py2.6.egg/django/core/servers/basehttp.py", line 321, in finish_response self.write(data) File "/Library/WebServer/Sites/django-4/lib/python2.6/site-packages/Django-1.3-py2.6.egg/django/core/servers/basehttp.py", line 400, in write self.send_headers() File "/Library/WebServer/Sites/django-4/lib/python2.6/site-packages/Django-1.3-py2.6.egg/django/core/servers/basehttp.py", line 465, in send_headers self.send_preamble() File "/Library/WebServer/Sites/django-4/lib/python2.6/site-packages/Django-1.3-py2.6.egg/django/core/servers/basehttp.py", line 382, in send_preamble 'Date: %s\r\n' % http_date() File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/socket.py", line 297, in write self.flush() File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/socket.py", line 284, in flush self._sock.sendall(buffer) error: [Errno 32] Broken pipe
This error arrises in:
def finish_response(self): """ Send any iterable data, then close self and the iterable Subclasses intended for use in asynchronous servers will want to redefine this method, such that it sets up callbacks in the event loop to iterate over the data, and to call 'self.close()' once the response is finished. """ if not self.result_is_file() or not self.sendfile(): for data in self.result: self.write(data) self.finish_content() self.close()
Ie., the write() call fails. The 'self.close()' at this point is bypassed and so close() on iterable is not called.
The second chance where could be called is handle_error() as called within except block of:
def run(self, application): """Invoke the application""" # Note to self: don't move the close()! Asynchronous servers shouldn't # call close() from finish_response(), so if you close() anywhere but # the double-error branch here, you'll break asynchronous servers by # prematurely closing. Async servers must return from 'run()' without # closing if there might still be output to iterate over. try: self.setup_environ() self.result = application(self.environ, self.start_response) self.finish_response() except: try: self.handle_error() except: # If we get an error handling an error, just give up already! self.close() raise # ...and let the actual server figure it out.
The code for that though is:
def handle_error(self): """Log current error, and send error output to client if possible""" self.log_exception(sys.exc_info()) if not self.headers_sent: self.result = self.error_output(self.environ, self.start_response) self.finish_response() # XXX else: attempt advanced recovery techniques for HTML or text?
Because though headers are already sent then 'self.close()' not called there either.
End result is that if application returned a custom iterable in some way, close() isn't called on it, meaning that one could have a leakage of resources when socket error occurs.
Code for django.core.servers.basehttp should be patched something like:
*** /Users/graham/Packages/Django-1.3/django/core/servers/basehttp.py 2011-03-02 21:40:48.000000000 +1100 --- /Library/WebServer/Sites/django-4/lib/python2.6/site-packages/Django-1.3-py2.6.egg/django/core/servers/basehttp.py 2011-06-13 17:21:56.000000000 +1000 *************** *** 283,294 **** self.result = application(self.environ, self.start_response) self.finish_response() except: ! try: ! self.handle_error() ! except: ! # If we get an error handling an error, just give up already! ! self.close() ! raise # ...and let the actual server figure it out. def setup_environ(self): """Set up the environment for one request""" --- 283,291 ---- self.result = application(self.environ, self.start_response) self.finish_response() except: ! self.handle_error() ! finally: ! self.close() def setup_environ(self): """Set up the environment for one request""" *************** *** 449,463 **** pass # XXX check if content-length was too short? def close(self): ! try: ! self.request_handler.log_request(self.status.split(' ',1)[0], self.bytes_sent) ! finally: try: ! if hasattr(self.result,'close'): ! self.result.close() finally: ! self.result = self.headers = self.status = self.environ = None ! self.bytes_sent = 0; self.headers_sent = False def send_headers(self): """Transmit headers to the client, via self._write()""" --- 446,461 ---- pass # XXX check if content-length was too short? def close(self): ! if self.result is not None: try: ! self.request_handler.log_request(self.status.split(' ',1)[0], self.bytes_sent) finally: ! try: ! if hasattr(self.result,'close'): ! self.result.close() ! finally: ! self.result = self.headers = self.status = self.environ = None ! self.bytes_sent = 0; self.headers_sent = False def send_headers(self): """Transmit headers to the client, via self._write()"""
Now, given that Django 1.4 does away with the internal implementation of a WSGI server and uses wsgiref instead, fully expect that this issue will be marked as 'will not fix' as no will care about it for older versions. :-)
This likely is also only going to be an issue where someone (like me) was screwing with things and wrapping the Django WSGI application object passed to the ServerHandler.run() method with some fancy WSGI middleware that was doing stuff that was dependent on close() always being called as it should be.
Thus, logged for posterity mostly.
Change History (7)
comment:1 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 12 years ago
The underlying bug was fixed in Python 2.7 two months ago: http://bugs.python.org/issue16220
comment:3 by , 12 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
In #19519 we're about to wire the request_finished
signal in BaseHttpResponse.close()
.
If we do that, that bug in Python <= 2.7.3 will have consequences on a document feature of Django, which justifies backporting the fix from Python.
comment:4 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 12 years ago
Has patch: | set |
---|
https://github.com/django/django/pull/612
(I will add the fix for #19519 to that pull request.)
comment:6 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Indeed, this issue no longer exists in Django 1.4.
Per the new backport policy (http://groups.google.com/group/django-developers/browse_thread/thread/3f856c6bccea172a) we don't have to fix it in Django 1.3: it's not a security issue, nor a data-loss bug, nor a crashing bug. The development server is not designed for environments where the resource leak may become an issue, so the issue will only pop up in some edge cases, like yours.
Django used a copy of wsgiref; I assume you've checked if the issue still exists in the standard library, and if it does, filed a bug in Python's tracker?