This is an automated email from the ASF dual-hosted git repository. jdanek pushed a commit to branch jd_tryout in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git
commit e3fffb205f2a2c590340de2db23335e6cbf65b32 Author: Jiri Daněk <jda...@redhat.com> AuthorDate: Sun Jan 30 02:37:49 2022 +0100 DISPATCH-2323 Simplify test HTTPServer shutdown and fix its leaks --- tests/http1_tests.py | 282 ++++++++++++++++++----------------- tests/system_tests_http1_adaptor.py | 21 ++- tests/system_tests_http1_over_tcp.py | 5 + 3 files changed, 167 insertions(+), 141 deletions(-) diff --git a/tests/http1_tests.py b/tests/http1_tests.py index dba13de..e3d671e 100644 --- a/tests/http1_tests.py +++ b/tests/http1_tests.py @@ -77,8 +77,8 @@ class RequestHandler(BaseHTTPRequestHandler): self.end_headers() self.wfile.write(b'Server Closed') self.wfile.flush() - self.close_connection = True - self.server.server_killed = True + self.close_connection = True # server will close connection from router + self.server.server_killed = True # server will not accept a reconnect return self._execute_request(self.server.system_tests["POST"]) @@ -136,15 +136,7 @@ class MyHTTPServer(HTTPServer): def __init__(self, addr, handler_cls, testcases): self.system_tests = testcases self.request_count = 0 - HTTPServer.__init__(self, addr, handler_cls) - - def server_close(self): - try: - # force immediate close of listening socket - self.socket.shutdown(socket.SHUT_RDWR) - except Exception: - pass - HTTPServer.server_close(self) + super().__init__(addr, handler_cls) class ThreadedTestClient: @@ -160,7 +152,6 @@ class ThreadedTestClient: self._logger = Logger(title="TestClient: %s" % self._id, print_to_console=False) self._thread = Thread(target=self._run) - self._thread.daemon = True self.error = None self.count = 0 self._thread.start() @@ -168,49 +159,50 @@ class ThreadedTestClient: def _run(self): self._logger.log("TestClient connecting on %s" % self._conn_addr) client = HTTPConnection(self._conn_addr, timeout=TIMEOUT) - self._logger.log("TestClient connected") - for loop in range(self._repeat): - self._logger.log("TestClient start request %d" % loop) - for op, tests in self._tests.items(): - for req, _, val in tests: - self._logger.log("TestClient sending %s %s request" % (op, req.target)) - req.send_request(client, - {"test-echo": "%s-%s-%s-%s" % (self._id, - loop, - op, - req.target)}) - self._logger.log("TestClient getting %s response" % op) - try: - rsp = client.getresponse() - except HTTPException as exc: - self._logger.log("TestClient response failed: %s" % exc) - self.error = str(exc) - return - self._logger.log("TestClient response %s received" % op) - if val: + try: + self._logger.log("TestClient connected") + for loop in range(self._repeat): + self._logger.log("TestClient start request %d" % loop) + for op, tests in self._tests.items(): + for req, _, val in tests: + self._logger.log("TestClient sending %s %s request" % (op, req.target)) + req.send_request(client, + {"test-echo": "%s-%s-%s-%s" % (self._id, + loop, + op, + req.target)}) + self._logger.log("TestClient getting %s response" % op) try: - body = val.check_response(rsp) - except Exception as exc: - self._logger.log("TestClient response invalid: %s" - % str(exc)) - self.error = "client failed: %s" % str(exc) + rsp = client.getresponse() + except HTTPException as exc: + self._logger.log("TestClient response failed: %s" % exc) + self.error = str(exc) return - - if req.method == "BODY" and body != b'': - self._logger.log("TestClient response invalid: %s" - % "body present!") - self.error = "error: body present!" - return - self.count += 1 - self._logger.log("TestClient request %s %s completed!" % - (op, req.target)) - client.close() - self._logger.log("TestClient to %s closed" % self._conn_addr) + self._logger.log("TestClient response %s received" % op) + if val: + try: + body = val.check_response(rsp) + except Exception as exc: + self._logger.log("TestClient response invalid: %s" + % str(exc)) + self.error = "client failed: %s" % str(exc) + return + + if req.method == "BODY" and body != b'': + self._logger.log("TestClient response invalid: %s" + % "body present!") + self.error = "error: body present!" + return + self.count += 1 + self._logger.log("TestClient request %s %s completed!" % + (op, req.target)) + finally: + client.close() + self._logger.log("TestClient to %s closed" % self._conn_addr) def wait(self, timeout=TIMEOUT): - self._thread.join(timeout=TIMEOUT) + self._thread.join(timeout=timeout) self._logger.log("TestClient %s shut down" % self._conn_addr) - sleep(0.5) # fudge factor allow socket close to complete def dump_log(self): self._logger.dump() @@ -251,8 +243,8 @@ class TestServer: handler_cls or RequestHandler, tests) self._server.allow_reuse_address = True + self._server.timeout = TIMEOUT self._thread = Thread(target=self._run) - self._thread.daemon = True self._thread.start() def _run(self): @@ -270,21 +262,32 @@ class TestServer: def wait(self, timeout=TIMEOUT): self._logger.log("TestServer %s:%s shutting down" % self._server_addr) self.request_count = 0 - if self._thread.is_alive(): - client = HTTPConnection("127.0.0.1:%s" % self._client_port, - timeout=TIMEOUT) + + self._send_shutdown_request() + self._thread.join(timeout=timeout) + self._server.server_close() + self.request_count = self._server.request_count + self._server = None + + def _send_shutdown_request(self): + """Sends a POST request instructing the test HTTPServer to shut down. + + This is necessary because the test HTTPServer cannot be interrupted while there is an open + connection to it. The incoming connection in question is from a qdrouterd that we keep up + for the duration of the entire testclass. The only place for server to graciously die is + after processing an incoming request and closing the connection, but before accepting a new one. + """ + client = HTTPConnection("127.0.0.1:%s" % self._client_port, + timeout=TIMEOUT) + try: client.putrequest("POST", "/SHUTDOWN") client.putheader("Content-Length", "0") client.endheaders() - # 13 == len('Server Closed') - client.getresponse().read(13) + with client.getresponse() as response: + body = response.read() + assert body == b'Server Closed', f"Unexpectedly, response was {body}" + finally: client.close() - self._thread.join(timeout=TIMEOUT) - if self._server: - self._server.server_close() - self.request_count = self._server.request_count - del self._server - sleep(0.5) # fudge factor allow socket close to complete def http1_ping(sport, cport): @@ -356,7 +359,7 @@ class RequestMsg: self.headers = headers or {} self.body = body - def send_request(self, conn, extra_headers=None): + def send_request(self, conn: HTTPConnection, extra_headers=None): extra_headers = extra_headers or {} conn.putrequest(self.method, self.target) for key, value in self.headers.items(): @@ -381,6 +384,9 @@ class ResponseValidator: self.expect_body = expect_body def check_response(self, rsp): + # always fully read the response first + body = rsp.read() + if self.status and rsp.status != self.status: raise Exception("Bad response code, expected %s got %s" % (self.status, rsp.status)) @@ -389,7 +395,6 @@ class ResponseValidator: raise Exception("Missing/bad header (%s), expected %s got %s" % (key, value, rsp.getheader(key))) - body = rsp.read() if self.expect_body and self.expect_body != body: raise Exception("Bad response body expected %s got %s" % (self.expect_body, body)) @@ -539,17 +544,19 @@ class CommonHttp1Edge2EdgeTest: ] } server = TestServer.new_server(self.http_server11_port, self.http_listener11_port, TESTS) - self.assertIsNotNone(server, TEST_SERVER_ERROR % self.http_server11_port) + try: + self.assertIsNotNone(server, TEST_SERVER_ERROR % self.http_server11_port) - self.EA2.wait_connectors() + self.EA2.wait_connectors() - client = ThreadedTestClient(TESTS, - self.http_listener11_port, - repeat=300) - client.wait() - self.assertIsNone(client.error) - self.assertEqual(300, client.count) - server.wait() + client = ThreadedTestClient(TESTS, + self.http_listener11_port, + repeat=300) + client.wait() + self.assertIsNone(client.error) + self.assertEqual(300, client.count) + finally: + server.wait() def test_03_server_reconnect(self): """ @@ -654,15 +661,17 @@ class CommonHttp1Edge2EdgeTest: # ensure links recover once the server re-appears server = TestServer.new_server(self.http_server11_port, self.http_listener11_port, TESTS) - self.assertIsNotNone(server, TEST_SERVER_ERROR % self.http_server11_port) + try: + self.assertIsNotNone(server, TEST_SERVER_ERROR % self.http_server11_port) - self.EA2.wait_connectors() + self.EA2.wait_connectors() - client = ThreadedTestClient(TESTS, self.http_listener11_port) - client.wait() - self.assertIsNone(client.error) - self.assertEqual(1, client.count) - server.wait() + client = ThreadedTestClient(TESTS, self.http_listener11_port) + client.wait() + self.assertIsNone(client.error) + self.assertEqual(1, client.count) + finally: + server.wait() def test_05_large_streaming_msg(self): """ @@ -1408,27 +1417,29 @@ class Http1CurlTestsMixIn: get_url = "http://%s:%s/GET/curl_get" % (host, port) head_url = "http://%s:%s/HEAD/curl_head" % (host, port) - status, out, err = run_curl(["--http1.1", "-G", get_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" - % (out, err)) + try: + status, out, err = run_curl(["--http1.1", "-G", get_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" + % (out, err)) - status, out, err = run_curl(["--http1.1", "-I", head_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("App-Header-2", out, "Unexpected out=%s (err=%s)" - % (out, err)) + status, out, err = run_curl(["--http1.1", "-I", head_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("App-Header-2", out, "Unexpected out=%s (err=%s)" + % (out, err)) - status, out, err = run_curl(["--http1.0", "-G", get_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" - % (out, err)) + status, out, err = run_curl(["--http1.0", "-G", get_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" + % (out, err)) - status, out, err = run_curl(["--http1.1", "-G", get_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" - % (out, err)) + status, out, err = run_curl(["--http1.1", "-G", get_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" + % (out, err)) - server.wait() + finally: + server.wait() @unittest.skipIf(not _curl_ok(), "required curl version >= %s" % str(CURL_VERSION)) @@ -1465,24 +1476,26 @@ class Http1CurlTestsMixIn: put_url = "http://%s:%s/PUT/curl_put" % (host, port) head_url = "http://%s:%s/HEAD/curl_head" % (host, port) - status, out, err = run_curl(["--http1.1", "-T", ".", put_url], - input="Mary had a little pug." - "\nIts fleece was brown as dirt." - "\nIts color made Mary shrug." - "\nShe should dress it in a shirt.") - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - - status, out, err = run_curl(["--http1.1", "-I", head_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("App-Header-2", out, "Unexpected out=%s (err=%s)" - % (out, err)) - - status, out, err = run_curl(["--http1.1", "-T", ".", put_url], - input="Ph'nglui mglw'nafh Cthulhu" - "\nR'lyeh wgah'nagl fhtagn") - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - - server.wait() + try: + status, out, err = run_curl(["--http1.1", "-T", ".", put_url], + input="Mary had a little pug." + "\nIts fleece was brown as dirt." + "\nIts color made Mary shrug." + "\nShe should dress it in a shirt.") + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + + status, out, err = run_curl(["--http1.1", "-I", head_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("App-Header-2", out, "Unexpected out=%s (err=%s)" + % (out, err)) + + status, out, err = run_curl(["--http1.1", "-T", ".", put_url], + input="Ph'nglui mglw'nafh Cthulhu" + "\nR'lyeh wgah'nagl fhtagn") + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + + finally: + server.wait() @unittest.skipIf(not _curl_ok(), "required curl version >= %s" % str(CURL_VERSION)) @@ -1522,22 +1535,23 @@ class Http1CurlTestsMixIn: post_url = "http://%s:%s/POST/curl_post" % (host, port) get_url = "http://%s:%s/GET/curl_get" % (host, port) - status, out, err = run_curl(["--http1.1", "-F", "name=Skupper", - "-F", "breed=Pug", post_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" - % (out, err)) - - status, out, err = run_curl(["--http1.1", "-G", get_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("0123456789", out, "Unexpected out=%s (err=%s)" - % (out, err)) - - status, out, err = run_curl(["--http1.1", "-F", "name=Coco", - "-F", "breed=French Bulldog", - post_url]) - self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) - self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" - % (out, err)) - - server.wait() + try: + status, out, err = run_curl(["--http1.1", "-F", "name=Skupper", + "-F", "breed=Pug", post_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" + % (out, err)) + + status, out, err = run_curl(["--http1.1", "-G", get_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("0123456789", out, "Unexpected out=%s (err=%s)" + % (out, err)) + + status, out, err = run_curl(["--http1.1", "-F", "name=Coco", + "-F", "breed=French Bulldog", + post_url]) + self.assertEqual(0, status, "curl error '%s' '%s'" % (out, err)) + self.assertIn("END OF TRANSMISSION", out, "Unexpected out=%s (err=%s)" + % (out, err)) + finally: + server.wait() diff --git a/tests/system_tests_http1_adaptor.py b/tests/system_tests_http1_adaptor.py index 49f3153..82c2382 100644 --- a/tests/system_tests_http1_adaptor.py +++ b/tests/system_tests_http1_adaptor.py @@ -208,13 +208,15 @@ class Http1AdaptorManagementTest(TestCase): self.assertEqual(1, len(e_mgmt.query(type=self.CONNECTOR_TYPE).results)) server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - server.bind(("", self.http_server_port)) - server.setblocking(True) - server.settimeout(5) - server.listen(1) - conn, _ = server.accept() - server.close() + try: + server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server.bind(("", self.http_server_port)) + server.setblocking(True) + server.settimeout(5) + server.listen(1) + conn, _ = server.accept() + finally: + server.close() # now check the interior router for the closest/http1Service address self.i_router.wait_address("closest/http1Service", subscribers=1) @@ -295,6 +297,11 @@ class Http1AdaptorOneRouterTest(Http1OneRouterTestBase, handler_cls=RequestHandler10) cls.INT_A.wait_connectors() + @classmethod + def tearDownClass(cls): + cls.http10_server.wait(TIMEOUT) + cls.http11_server.wait(TIMEOUT) + def test_005_get_10(self): client = HTTPConnection("127.0.0.1:%s" % self.http_listener10_port, timeout=TIMEOUT) diff --git a/tests/system_tests_http1_over_tcp.py b/tests/system_tests_http1_over_tcp.py index 2483fe9..2e63b35 100644 --- a/tests/system_tests_http1_over_tcp.py +++ b/tests/system_tests_http1_over_tcp.py @@ -70,6 +70,11 @@ class Http1OverTcpOneRouterTest(Http1OneRouterTestBase, handler_cls=RequestHandler10) cls.INT_A.wait_connectors() + @classmethod + def tearDownClass(cls): + cls.http10_server.wait() + cls.http11_server.wait() + class Http1OverTcpEdge2EdgeTest(Http1Edge2EdgeTestBase, CommonHttp1Edge2EdgeTest): """ --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org