Socket timeout

[Augie Fackler]

> On Oct 3, 2018, at 12:38, Cédric Krier <cedric.krier at b2ck.com> wrote:
> 
> On 2018-10-03 11:52, Augie Fackler wrote:
>>> On Sep 29, 2018, at 05:32, Cédric Krier <cedric.krier at b2ck.com> wrote:
>>> On 2018-09-24 08:42, Augie Fackler wrote:
>>>> Greg, have you observed anyplace we could configure some global timeouts like this user is requesting? It sounds reasonable to me...
>>> 
>>> I made some test and indeed there is no timeout when using HTTP
>>> protocol.
>>> As far as I can see, HTTP connections are made with the standard library
>>> httplib.HTTPConnection [1]. If no timeout is given which seems to be the
>>> case, the global default timeout is used [2] and it is None by default.
>>> So I think it will be good to have a configuration to set the default
>>> timeout in dispatch.py. If you agree on this, I can work on a patch.
>> 
>> I might be inclined to do this as a context manager in httppeer or some lower layer, so we can restore it when done, just to avoid as many surprises as possible.
> 
> Does you mean something like this:
> 

Sure! send that as a patch to mercurial-devel@ or use phabricator?

> 
> diff -r 707c3804e607 mercurial/keepalive.py
> --- a/mercurial/keepalive.py	Fri Sep 28 12:56:57 2018 -0700
> +++ b/mercurial/keepalive.py	Wed Oct 03 18:36:49 2018 +0200
> @@ -172,8 +172,9 @@
>             return dict(self._hostmap)
> 
> class KeepAliveHandler(object):
> -    def __init__(self):
> +    def __init__(self, timeout=None):
>         self._cm = ConnectionManager()
> +        self._timeout = timeout
> 
>     #### Connection Management
>     def open_connections(self):
> @@ -232,7 +233,7 @@
>                 h = self._cm.get_ready_conn(host)
>             else:
>                 # no (working) free connections were found.  Create a new one.
> -                h = http_class(host)
> +                h = http_class(host, timeout=self._timeout)
>                 if DEBUG:
>                     DEBUG.info("creating new connection to %s (%d)",
>                                host, id(h))
> diff -r 707c3804e607 mercurial/url.py
> --- a/mercurial/url.py	Fri Sep 28 12:56:57 2018 -0700
> +++ b/mercurial/url.py	Wed Oct 03 18:36:49 2018 +0200
> @@ -317,8 +317,8 @@
> 
> class logginghttphandler(httphandler):
>     """HTTP handler that logs socket I/O."""
> -    def __init__(self, logfh, name, observeropts):
> -        super(logginghttphandler, self).__init__()
> +    def __init__(self, logfh, name, observeropts, timeout=None):
> +        super(logginghttphandler, self).__init__(timeout=timeout)
> 
>         self._logfh = logfh
>         self._logname = name
> @@ -365,8 +365,8 @@
>             sslutil.validatesocket(self.sock)
> 
>     class httpshandler(keepalive.KeepAliveHandler, urlreq.httpshandler):
> -        def __init__(self, ui):
> -            keepalive.KeepAliveHandler.__init__(self)
> +        def __init__(self, ui, timeout=None):
> +            keepalive.KeepAliveHandler.__init__(self, timeout=timeout)
>             urlreq.httpshandler.__init__(self)
>             self.ui = ui
>             self.pwmgr = passwordmgr(self.ui,
> @@ -525,18 +525,19 @@
>     ``sendaccept`` allows controlling whether the ``Accept`` request header
>     is sent. The header is sent by default.
>     '''
> +    timeout = ui.configwith(float, 'socket', 'timeout', default=5)
>     handlers = []
> 
>     if loggingfh:
>         handlers.append(logginghttphandler(loggingfh, loggingname,
> -                                           loggingopts or {}))
> +                                           loggingopts or {}, timeout=timeout))
>         # We don't yet support HTTPS when logging I/O. If we attempt to open
>         # an HTTPS URL, we'll likely fail due to unknown protocol.
> 
>     else:
> -        handlers.append(httphandler())
> +        handlers.append(httphandler(timeout=timeout))
>         if has_https:
> -            handlers.append(httpshandler(ui))
> +            handlers.append(httpshandler(ui, timeout=timeout))
> 
>     handlers.append(proxyhandler(ui))
> 
> 
> 
> -- 
> Cédric Krier - B2CK SPRL
> Email/Jabber: cedric.krier at b2ck.com
> Tel: +32 472 54 46 59
> Website: http://www.b2ck.com/