https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/221196
Fixes #217733. Observed only on Windows on Arm but I bet it can happen other places. Sometimes the test would fail because it did not find the error message in the log file. The problem is that the test is relying on the client to realise the timeout has been exceeded and close the connection, before the fake responder can send back status 200. So what sometimes happens is: * The client sends the request. * The server gets the request and waits N seconds. * The client is delayed doing other things, and doesn't realise the timeout has been reached. So the connection is still open. * The server tries to respond with 200, and that sends succesfully. * The client wakes up and sees the 200 response, so there's no error to report. To fix this, I've changed the responder from merely being slow to never responding. This means the client can be delayed as long as it likes. To prevent the responder hanging around, it waits on an event which the test will set once the test is done (whether pass or fail), then the responder will shut down normally. This change does mean that if the client was buggy, we could be waiting up to the global test timeout. However I think that's a reasonable trade to not have flaky test results. The alternative is to increase the responder delay, which will have to vary machine to machine and run to run. In the end, you'd end up with a delay very similar to the global test timeout anyway. >From f07428604040ef211b4e4612233bd6ed07fd781d Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 4 Sep 2026 10:43:54 +0000 Subject: [PATCH] [lldb][test] Fix flakey results from TestSymStore.py test_http_timeout Fixes #217733. Observed only on Windows on Arm but I bet it can happen other places. Sometimes the test would fail because it did not find the error message in the log file. The problem is that the test is relying on the client to realise the timeout has been exceeded and close the connection, before the fake responder can send back status 200. So what sometimes happens is: * The client sends the request. * The server gets the request and waits N seconds. * The client is delayed doing other things, and doesn't realise the timeout has been reached. So the connection is still open. * The server tries to respond with 200, and that sends succesfully. * The client wakes up and sees the 200 response, so there's no error to report. To fix this, I've changed the responder from merely being slow to never responding. This means the client can be delayed as long as it likes. To prevent the responder hanging around, it waits on an event which the test will set once the test is done (whether pass or fail), then the responder will shut down normally. This change does mean that if the client was buggy, we could be waiting up to the global test timeout. However I think that's a reasonable trade to not have flakey test results. The alternative is to increase the responder delay, which will have to vary machine to machine and run to run. In the end, you'd end up with a delay very similar to the global test timeout anyway. --- lldb/test/API/symstore/TestSymStore.py | 40 +++++++++++++------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/lldb/test/API/symstore/TestSymStore.py b/lldb/test/API/symstore/TestSymStore.py index 4c668134447d1..09cd5a71e715b 100644 --- a/lldb/test/API/symstore/TestSymStore.py +++ b/lldb/test/API/symstore/TestSymStore.py @@ -234,18 +234,13 @@ def do_GET(self): super().do_GET() -class SlowHTTPHandler(http.server.BaseHTTPRequestHandler): - """HTTP request handler that delays responses to simulate a slow server.""" +class NeverRespondsHTTPHandler(http.server.BaseHTTPRequestHandler): + """HTTP request handler that never responds.""" - delay = 2 # seconds to sleep before responding; set per test + can_finish = threading.Event() def do_GET(self): - try: - time.sleep(self.delay) - self.send_response(200) - self.end_headers() - except (BrokenPipeError, ConnectionResetError): - pass # client disconnected after timeout, as expected + self.can_finish.wait() def log_message(self, *args): pass # suppress server-side output @@ -451,18 +446,23 @@ def test_http_timeout(self): """ exe, sym = self.build_inferior() with MockedSymStore(self, exe, sym) as dir: - SlowHTTPHandler.delay = 5 # seconds; exceeds the 1s timeout below + NeverRespondsHTTPHandler.can_finish.clear() self.runCmd("settings set plugin.symbol-locator.symstore.timeout 1") - with HTTPServer(dir, SlowHTTPHandler) as url: - self.runCmd(f"settings set plugin.symbol-locator.symstore.urls {url}") - warnings = "" - with open(self.getBuildArtifact("stderr.txt"), "w+b") as err_file: - self.dbg.SetErrorFileHandle(err_file, False) - self.try_breakpoint(exe, should_have_loc=False) - self.dbg.SetErrorFileHandle(sys.stderr, False) - err_file.seek(0) - warnings = err_file.read().decode() - self.assertIn("failed to download", warnings) + + with HTTPServer(dir, NeverRespondsHTTPHandler) as url: + try: + self.runCmd( + f"settings set plugin.symbol-locator.symstore.urls {url}" + ) + with open(self.getBuildArtifact("stderr.txt"), "w+b") as err_file: + self.dbg.SetErrorFileHandle(err_file, False) + self.try_breakpoint(exe, should_have_loc=False) + self.dbg.SetErrorFileHandle(sys.stderr, False) + err_file.seek(0) + warnings = err_file.read().decode() + self.assertIn("failed to download", warnings) + finally: + NeverRespondsHTTPHandler.can_finish.set() @skipUnlessPackageAvailable("cryptography") def test_https(self): _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
