Charles-François Natali <[email protected]> added the comment:
Thanks for the report.
Several things are going on here:
1. Even though socketserver's StreamRequestHandler uses unbuffered wfile for
the socket
"""
class StreamRequestHandler(BaseRequestHandler):
[...]
rbufsize = -1
wbufsize = 0
# A timeout to apply to the request socket, if not None.
timeout = None
# Disable nagle algorithm for this socket, if True.
# Use only when wbufsize != 0, to avoid small packets.
disable_nagle_algorithm = False
def setup(self):
self.connection = self.request
if self.timeout is not None:
self.connection.settimeout(self.timeout)
if self.disable_nagle_algorithm:
self.connection.setsockopt(socket.IPPROTO_TCP,
socket.TCP_NODELAY, True)
self.rfile = self.connection.makefile('rb', self.rbufsize)
self.wfile = self.connection.makefile('wb', self.wbufsize)
"""
data is internally buffered by socket._fileobject:
"""
def write(self, data):
data = str(data) # XXX Should really reject non-string non-buffers
if not data:
return
self._wbuf.append(data)
self._wbuf_len += len(data)
if (self._wbufsize == 0 or
self._wbufsize == 1 and '\n' in data or
self._wbuf_len >= self._wbufsize):
self.flush()
"""
Usually it doesn't turn out to be a problem because if the object is unbuffered
the buffer is flushed right away, but in this specific case, it's a problem
because a subsequent call to flush() will try to drain the data buffered
temporarily, which triggers the second EPIPE from the
StreamRequestHandler.finish()
Note that Python 3.3 doesn't have this problem.
While this is arguably a bad behavior, I don't feel comfortable changing this
in 2.7 (either by changing the write() and flush() method or by just checking
that the _fileobject is indeed buffered before flushing it).
Moreover, this wouldn't solve the problem at hand in case the user chose to
use a buffered connection (StreamRequestHandler.wbufsize > 0).
2. I think the root cause of the problem is that the handler's finish() method
is called even when an exception occured during the handler, in which case
nothing can be assumed about the state of the connection:
"""
class BaseRequestHandler:
[...]
self.setup()
try:
self.handle()
finally:
self.finish()
"""
Which is funny, because it doesn't match the documentation:
"""
.. method:: RequestHandler.finish()
Called after the :meth:`handle` method to perform any clean-up actions
required. The default implementation does nothing. If :meth:`setup` or
:meth:`handle` raise an exception, this function will not be called.
"""
So the obvious solution would be to change the code to match the documentation,
and not call finish when an exception was raised.
But that would be a behavior change, and could introduce resource leaks.
For example, here's StreamRequestHandler finish() method:
"""
def finish(self):
if not self.wfile.closed:
self.wfile.flush()
self.wfile.close()
self.rfile.close()
"""
While in this specific case if wouldn't lead to a FD leak (because the
underlying socket is closed by the server code), one could imagine a case where
it could have a negative impact, so I'm not sure about changing this.
Finally, you could get rid of this error by overriding
StreamRequestHandler.finish() method or catching the first exception in the
handle() method and close the connection explicitely.
So I'd like to know what others think about this :-)
----------
nosy: +pitrou
_______________________________________
Python tracker <[email protected]>
<http://bugs.python.org/issue14574>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
