[Twisted-Python] Submitted for your consideration

[Jp Calderone]

On Wed, Jun 11, 2003 at 08:27:52PM +1000, Andrew Bennetts wrote:
> On Wed, Jun 11, 2003 at 05:52:16AM -0400, Jp Calderone wrote:
> >   Attached is a patch to add timeout support to t.web (via a minor
> > HTTPChannel change).
> > 
> >   The performance impact should be light, though it does impose an extra
> > function call per received line.  If this is found to unacceptable, I do not
> > thing it would be unreasonable to change the "__lastReceived" attribute's
> > name, document it as public, and require the update be made inline.
> 
> [I've already made this point on IRC, but anyway...]
> 
> It also will timeout clients if a request takes a long time to run.  Think
> streaming web pages, e.g. LivePage, or a streaming log from a *very* slow
> buildbot build.  This patch should use .setTimeout(None) to disable timeouts
> while a request is being processed (and then of course re-enable timeouts
> once all outstanding requests have been serviced).  I don't like the idea of
> disconnecting a client simply because the server is taking a long time to
> serve the request, or because a client has a slow link.
> 
> I realise the default timeout is 12 hours, but:
>     - I can imagine cases where HTTP connections last longer than 12 hours
>       without receiving traffic from the client, e.g. streaming HTTP, where
>       the client sends nothing, but the server keeps streaming buildbot
>       results/javascript events/music, or simply a very long running request
>       (i.e. because of an unusually massive DB query, or something).  You'd
>       *really* be annoyed if your query took 13 hours, only to discover
>       after 12 hours Twisted Web arbitrarily decided your connection was
>       idle and killed it.  
>     - Another scenario: A dial-up user downloading an ISO of their favorite
>       Linux distro.  This is a legitimate request, and shouldn't be
>       arbitrarily cut-off halfway.  If a client wants to download 600Mb at
>       5kB/s, Twisted Web should let them.
>     - It should still work correctly if the default is changed.  Someone
>       might be paranoid, and set the timeout down to, say, 5 minutes.  This
>       shouldn't cause adverse affects (beyond killing connections that have
>       been idle that long).


  These are good points.  Thanks for making them on the ML :)  Here is a
modified patch (hopefully modified in the proper way).

  Jp
-------------- next part --------------
Index: protocols/http.py
===================================================================
RCS file: /cvs/Twisted/twisted/protocols/http.py,v
retrieving revision 1.80
diff -u -r1.80 http.py
--- protocols/http.py	17 May 2003 20:54:12 -0000	1.80
+++ protocols/http.py	11 Jun 2003 11:36:00 -0000
@@ -47,6 +47,7 @@
 
 # twisted imports
 from twisted.internet import interfaces, reactor, protocol
+from twisted.protocols import policies
 from twisted.python import log
 
 
@@ -856,7 +857,7 @@
         pass
 
 
-class HTTPChannel(basic.LineReceiver):
+class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
     """A receiver for HTTP requests."""
 
     length = 0
@@ -868,12 +869,20 @@
     # set in instances or subclasses
     requestFactory = Request
 
+    # Timeout connections after 12 hours of inactivity
+    timeOut = 60 * 60 * 12
+    _savedTimeOut = None
 
     def __init__(self):
         # the request queue
         self.requests = []
 
+    def connectionMade(self):
+        self.setTimeout(self.timeOut)
+    
     def lineReceived(self, line):
+        self.resetTimeout()
+
         if self.__first_line:
             # if this connection is not persistent, drop any data which
             # the client (illegally) sent after the last request.
@@ -942,6 +951,11 @@
         self.__first_line = 1
         del self._command, self._path, self._version
 
+        # Disable the idle timeout, in case this request takes a long
+        # time to finish generating output.
+        if self.timeOut:
+            self._savedTimeOut = self.setTimeout(None)
+
         req = self.requests[-1]
         req.requestReceived(command, path, version)
 
@@ -1000,10 +1014,14 @@
             # notify next request it can start writing
             if self.requests:
                 self.requests[0].noLongerQueued()
+            else:
+                if self._savedTimeOut:
+                    self.setTimeout(self._savedTimeOut)
         else:
             self.transport.loseConnection()
 
     def connectionLost(self, reason):
+        self.setTimeout(None)
         for request in self.requests:
             request.connectionLost(reason)
 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: </pipermail/twisted-python/attachments/20030611/69d758ac/attachment.sig>