Try a different approach to timeouts: dynamic, instead of hardcoded.

This commit is contained in:
Itamar Turner-Trauring 2022-11-21 12:24:50 -05:00
parent 8cfdae2ab4
commit 3a613aee70
2 changed files with 34 additions and 17 deletions

View File

@ -20,6 +20,7 @@ from foolscap.api import flushEventualQueue
from allmydata import client from allmydata import client
from allmydata.introducer.server import create_introducer from allmydata.introducer.server import create_introducer
from allmydata.util import fileutil, log, pollmixin from allmydata.util import fileutil, log, pollmixin
from allmydata.util.deferredutil import async_to_deferred
from allmydata.storage import http_client from allmydata.storage import http_client
from allmydata.storage_client import ( from allmydata.storage_client import (
NativeStorageServer, NativeStorageServer,
@ -639,6 +640,33 @@ def _render_section_values(values):
)) ))
@async_to_deferred
async def spin_until_cleanup_done(value=None, timeout=10):
"""
At the end of the test, spin until either a timeout is hit, or the reactor
has no more DelayedCalls.
Make sure to register during setUp.
"""
def num_fds():
if hasattr(reactor, "handles"):
# IOCP!
return len(reactor.handles)
else:
# Normal reactor
return len([r for r in reactor.getReaders()
if r.__class__.__name__ not in ("_UnixWaker", "_SIGCHLDWaker")]
) + len(reactor.getWriters())
for i in range(timeout * 1000):
# There's a single DelayedCall for AsynchronousDeferredRunTest's
# timeout...
if (len(reactor.getDelayedCalls()) < 2 and num_fds() == 0):
break
await deferLater(reactor, 0.001)
return value
class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin): class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
# If set to True, use Foolscap for storage protocol. If set to False, HTTP # If set to True, use Foolscap for storage protocol. If set to False, HTTP
@ -685,7 +713,7 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
d = self.sparent.stopService() d = self.sparent.stopService()
d.addBoth(flush_but_dont_ignore) d.addBoth(flush_but_dont_ignore)
d.addBoth(lambda x: self.close_idle_http_connections().addCallback(lambda _: x)) d.addBoth(lambda x: self.close_idle_http_connections().addCallback(lambda _: x))
d.addBoth(lambda x: deferLater(reactor, 2, lambda: x)) d.addBoth(spin_until_cleanup_done)
return d return d
def getdir(self, subdir): def getdir(self, subdir):

View File

@ -12,7 +12,6 @@ from cryptography import x509
from twisted.internet.endpoints import serverFromString from twisted.internet.endpoints import serverFromString
from twisted.internet import reactor from twisted.internet import reactor
from twisted.internet.task import deferLater
from twisted.web.server import Site from twisted.web.server import Site
from twisted.web.static import Data from twisted.web.static import Data
from twisted.web.client import Agent, HTTPConnectionPool, ResponseNeverReceived from twisted.web.client import Agent, HTTPConnectionPool, ResponseNeverReceived
@ -30,6 +29,7 @@ from ..storage.http_common import get_spki_hash
from ..storage.http_client import _StorageClientHTTPSPolicy from ..storage.http_client import _StorageClientHTTPSPolicy
from ..storage.http_server import _TLSEndpointWrapper from ..storage.http_server import _TLSEndpointWrapper
from ..util.deferredutil import async_to_deferred from ..util.deferredutil import async_to_deferred
from .common_system import spin_until_cleanup_done
class HTTPSNurlTests(SyncTestCase): class HTTPSNurlTests(SyncTestCase):
@ -87,6 +87,10 @@ class PinningHTTPSValidation(AsyncTestCase):
self.addCleanup(self._port_assigner.tearDown) self.addCleanup(self._port_assigner.tearDown)
return AsyncTestCase.setUp(self) return AsyncTestCase.setUp(self)
def tearDown(self):
AsyncTestCase.tearDown(self)
return spin_until_cleanup_done()
@asynccontextmanager @asynccontextmanager
async def listen(self, private_key_path: FilePath, cert_path: FilePath): async def listen(self, private_key_path: FilePath, cert_path: FilePath):
""" """
@ -107,9 +111,6 @@ class PinningHTTPSValidation(AsyncTestCase):
yield f"https://127.0.0.1:{listening_port.getHost().port}/" yield f"https://127.0.0.1:{listening_port.getHost().port}/"
finally: finally:
await listening_port.stopListening() await listening_port.stopListening()
# Make sure all server connections are closed :( No idea why this
# is necessary when it's not for IStorageServer HTTPS tests.
await deferLater(reactor, 0.01)
def request(self, url: str, expected_certificate: x509.Certificate): def request(self, url: str, expected_certificate: x509.Certificate):
""" """
@ -144,10 +145,6 @@ class PinningHTTPSValidation(AsyncTestCase):
response = await self.request(url, certificate) response = await self.request(url, certificate)
self.assertEqual(await response.content(), b"YOYODYNE") self.assertEqual(await response.content(), b"YOYODYNE")
# We keep getting TLSMemoryBIOProtocol being left around, so try harder
# to wait for it to finish.
await deferLater(reactor, 0.01)
@async_to_deferred @async_to_deferred
async def test_server_certificate_has_wrong_hash(self): async def test_server_certificate_has_wrong_hash(self):
""" """
@ -183,10 +180,6 @@ class PinningHTTPSValidation(AsyncTestCase):
response = await self.request(url, certificate) response = await self.request(url, certificate)
self.assertEqual(await response.content(), b"YOYODYNE") self.assertEqual(await response.content(), b"YOYODYNE")
# We keep getting TLSMemoryBIOProtocol being left around, so try harder
# to wait for it to finish.
await deferLater(reactor, 0.01)
@async_to_deferred @async_to_deferred
async def test_server_certificate_not_valid_yet(self): async def test_server_certificate_not_valid_yet(self):
""" """
@ -206,10 +199,6 @@ class PinningHTTPSValidation(AsyncTestCase):
response = await self.request(url, certificate) response = await self.request(url, certificate)
self.assertEqual(await response.content(), b"YOYODYNE") self.assertEqual(await response.content(), b"YOYODYNE")
# We keep getting TLSMemoryBIOProtocol being left around, so try harder
# to wait for it to finish.
await deferLater(reactor, 0.001)
# A potential attack to test is a private key that doesn't match the # A potential attack to test is a private key that doesn't match the
# certificate... but OpenSSL (quite rightly) won't let you listen with that # certificate... but OpenSSL (quite rightly) won't let you listen with that
# so I don't know how to test that! See # so I don't know how to test that! See