Edgewall Software
Modify

Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#9103 closed defect (fixed)

req.write can trigger spurious errors related to client disconnects

Reported by: Christian Boos Owned by: Christian Boos
Priority: normal Milestone: 0.11.8
Component: web frontend Version: 0.11-stable
Severity: normal Keywords: broken pipe connection reset
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

A bit similar to #3957, but different place.

Traceback (most recent call last):
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/main.py", line 450, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/main.py", line 230, in dispatch
    req.send(output, content_type or 'text/html')
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/api.py", line 359, in send
    self.write(content)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/api.py", line 463, in write
    self._write(data)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 1229, in write
    req.stdout.write(data)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 327, in write
    self._write(data)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 311, in _write
    self._conn.writeRecord(rec)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 709, in writeRecord
    rec.write(self._sock)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 542, in write
    self._sendall(sock, header)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 519, in _sendall
    sent = sock.send(data)
error: (32, 'Broken pipe')
2010-01-09 07:44:29,070 Trac[main] ERROR: Internal Server Error:
Traceback (most recent call last):
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/main.py", line 450, in _dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/main.py", line 230, in dispatch
    req.send(output, content_type or 'text/html')
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/api.py", line 359, in send
    self.write(content)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/api.py", line 463, in write
    self._write(data)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 1229, in write
    req.stdout.write(data)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 327, in write
    self._write(data)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 311, in _write
    self._conn.writeRecord(rec)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 709, in writeRecord
    rec.write(self._sock)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 542, in write
    self._sendall(sock, header)
  File "/usr/local/virtualenv-0.11/lib/python2.5/site-packages/Trac-0.11.7stable_r8997-py2.5.egg/trac/web/_fcgi.py", line 519, in _sendall
    sent = sock.send(data)
error: (104, 'Connection reset by peer')

0.12 version:

2010-03-02 10:32:17,183 Trac[main] ERROR: Internal Server Error:
Traceback (most recent call last):
  File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 499, in _dispatch_request
    dispatcher.dispatch(req)
  File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 256, in dispatch
    req.send(output, content_type or 'text/html')
  File "build/bdist.linux-x86_64/egg/trac/web/api.py", line 375, in send
    self.write(content)
  File "build/bdist.linux-x86_64/egg/trac/web/api.py", line 494, in write
    self._write(data)
  File "build/bdist.linux-x86_64/egg/trac/web/_fcgi.py", line 1229, in write
    req.stdout.write(data)
  File "build/bdist.linux-x86_64/egg/trac/web/_fcgi.py", line 327, in write
    self._write(data)
  File "build/bdist.linux-x86_64/egg/trac/web/_fcgi.py", line 311, in _write
    self._conn.writeRecord(rec)
  File "build/bdist.linux-x86_64/egg/trac/web/_fcgi.py", line 709, in writeRecord
    rec.write(self._sock)
  File "build/bdist.linux-x86_64/egg/trac/web/_fcgi.py", line 542, in write
    self._sendall(sock, header)
  File "build/bdist.linux-x86_64/egg/trac/web/_fcgi.py", line 520, in _sendall
    sent = sock.send(data)
error: (32, 'Broken pipe')

As discussed in ticket:9077#comment:4, point 1.

Attachments (0)

Change History (11)

comment:1 by Christian Boos, 14 years ago

  • trac/web/api.py

    diff --git a/trac/web/api.py b/trac/web/api.py
    a b class Request(object):  
    462462            self.end_headers()
    463463        if isinstance(data, unicode):
    464464            data = data.encode(self._outcharset or 'utf-8')
    465         self._write(data)
     465        try:
     466            self._write(data)
     467        except (IOError, socket.error), e:
     468            if e.args[0] not in (errno.EPIPE, errno.ECONNRESET, 10053, 10054):
     469                raise
    466470
    467471    # Internal methods

Seems to work, more testing needed.

comment:2 by Remy Blank, 14 years ago

Unfortunately, I cannot reproduce the issue with a browser, even without the patch. But I observe no difference in behavior with the patch applied, so it doesn't seem to have a negative effect.

But instead of swallowing the exception silently (as in the patch), and letting the request terminate "normally", shouldn't we rather abort the request by raising a RequestDone? The result of the request is unusable anyway, and subsequent calls to req.write() will raise the same exception again. Or do we risk skipping over wanted side-effects in the rest of the request?

in reply to:  2 comment:3 by Christian Boos, 14 years ago

Replying to rblank:

But instead of swallowing the exception silently (as in the patch), and letting the request terminate "normally", shouldn't we rather abort the request by raising a RequestDone? The result of the request is unusable anyway, and subsequent calls to req.write() will raise the same exception again.

Yes, that was my follow-up questioning as well.

  • trac/web/api.py

    diff --git a/trac/web/api.py b/trac/web/api.py
    a b class Request(object):  
    326326        self.send_header('Cache-control', 'no-cache')
    327327        self.send_header('Expires', 'Fri, 01 Jan 1999 00:00:00 GMT')
    328328        self.end_headers()
    329 
    330329        raise RequestDone
    331330
    332331    def display(self, template, content_type='text/html', status=200):
    class Request(object):  
    465464        try:
    466465            self._write(data)
    467466        except (IOError, socket.error), e:
    468             if e.args[0] not in (errno.EPIPE, errno.ECONNRESET, 10053, 10054):
    469                 raise
     467            if e.args[0] in (errno.EPIPE, errno.ECONNRESET, 10053, 10054):
     468                raise RequestDone
     469            raise
    470470
    471471    # Internal methods

Or do we risk skipping over wanted side-effects in the rest of the request?

I don't think so, as writing some content is usually the very last thing that happens, at the end of processing a request. And even if this would happen, I think that would fit with the "Cancel" semantic of an aborted read or a connection reset.

comment:4 by Christian Boos, 14 years ago

Resolution: fixed
Status: newclosed

Patch above committed as r9335.

comment:5 by Christian Boos, 14 years ago

Resolution: fixed
Status: closedreopened

Argh, neither socket nor errno were imported here…

comment:6 by Christian Boos, 14 years ago

Resolution: fixed
Status: reopenedclosed

Fixed in r9391, will be in patch release 0.11.7.1.

comment:7 by Remy Blank, 14 years ago

Don't forget to backport it to 0.11-stable.

comment:8 by Christian Boos, 14 years ago

Done in r9403, thanks for the reminder I had forgotten, indeed ;-)

comment:9 by Thijs Triemstra <lists@…>, 14 years ago

Also ran into this one, switching to 0.11-stable.

in reply to:  9 comment:10 by Christian Boos, 14 years ago

Replying to Thijs Triemstra <lists@…>:

Also ran into this one, switching to 0.11-stable.

What do you mean, r9403 was not enough? If that's the case, please post the full backtrace.

comment:11 by Christian Boos, 14 years ago

Milestone: 0.11.70.11.7.1

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.