From b67648aa666345b0800b48f6c203538b21c9a201 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Mon, 8 Jun 2015 15:18:00 -0300 Subject: [bug] Makes request method respect a hard limit Altough we specify maxPersistentPerHost, Twisted won't stop opening connections after that. This limit is used just to keep the size of persistent connections pool under control. Additional connections will be made as non persistent. So, if we ask 10000 requests, it will open 10000 connections immediately and leave 10 open after all finished. For checking this behavior, see getConnection from Twisted source: http://twistedmatrix.com/trac/browser/tags/releases/twisted-15.2.1/twisted/web/client.py#L1203 I tested this by using http_target from soledad without a local database to download all encrypted docs from one account with 1700 of them. The program just hangs and crashes with 1000+ connections and "Too many files open" warnings. With this fix, it was able to download normally, respecting the maxPersistentPerHost as a limiter. :) --- src/leap/common/http.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/leap/common/http.py b/src/leap/common/http.py index 1e384e5..d4a214c 100644 --- a/src/leap/common/http.py +++ b/src/leap/common/http.py @@ -35,6 +35,7 @@ from leap.common.certs import get_compatible_ssl_context_factory from zope.interface import implements from twisted.internet import reactor +from twisted.internet import defer from twisted.internet.defer import succeed from twisted.web.client import Agent @@ -56,6 +57,13 @@ class HTTPClient(object): """ HTTP client done the twisted way, with a main focus on pinning the SSL certificate. + + By default, it uses a shared connection pool. If you want a dedicated + one, create and pass on __init__ pool parameter. + Please note that this client will limit the maximum amount of connections + by using a DeferredSemaphore. + This limit is equal to the maxPersistentPerHost used on pool and is needed + in order to avoid resource abuse on huge requests batches. """ def __init__(self, cert_file=None, pool=_pool): @@ -65,6 +73,9 @@ class HTTPClient(object): :param cert_file: The path to the certificate file, if None given the system's CAs will be used. :type cert_file: str + :param pool: An optional dedicated connection pool to override the + default shared one. + :type pool: HTTPConnectionPool """ policy = get_compatible_ssl_context_factory(cert_file) @@ -73,6 +84,7 @@ class HTTPClient(object): reactor, policy, pool=pool) + self._semaphore = defer.DeferredSemaphore(pool.maxPersistentPerHost) def request(self, url, method='GET', body=None, headers={}): """ @@ -92,8 +104,9 @@ class HTTPClient(object): """ if body: body = HTTPClient.StringBodyProducer(body) - d = self._agent.request( - method, url, headers=Headers(headers), bodyProducer=body) + d = self._semaphore.run(self._agent.request, + method, url, headers=Headers(headers), + bodyProducer=body) d.addCallback(readBody) return d -- cgit v1.2.3