[Lldb-commits] [lldb] [lldb] Make timeout for HTTP send/receive in SymbolLocatorSymStore configurable (PR #192061)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Stefan Gränitz (weliveindetail)

<details>
<summary>Changes</summary>

This patch adds the `plugin.symbol-locator.symstore.timeout` setting and keeps the previously hard-coded value of 60 sections as the default. It adds a test to check that the breakpoint doesn't resolve and we get the correct warning message if the timeout hits.

---
Full diff: https://github.com/llvm/llvm-project/pull/192061.diff


3 Files Affected:

- (modified) lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp (+8-3) 
- (modified) lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td (+3) 
- (modified) lldb/test/API/symstore/TestSymStore.py (+39) 


``````````diff
diff --git a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp
index 41b15c6725a26..2f78548702225 100644
--- a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp
+++ b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStore.cpp
@@ -58,6 +58,12 @@ class PluginProperties : public Properties {
     m_collection_sp->GetPropertyAtIndexAsArgs(ePropertySymStoreURLs, urls);
     return urls;
   }
+
+  uint64_t GetTimeout() const {
+    const uint32_t idx = ePropertyTimeout;
+    return GetPropertyAtIndexAs<uint64_t>(
+        idx, g_symbollocatorsymstore_properties[idx].default_uint_value);
+  }
 };
 
 } // namespace
@@ -168,9 +174,8 @@ RequestFileFromSymStoreServerHTTP(llvm::StringRef base_url, llvm::StringRef key,
   }
 
   llvm::HTTPClient client;
-  // TODO: Since PDBs can be huge, we should distinguish between resolve,
-  // connect, send and receive.
-  client.setTimeout(std::chrono::seconds(60));
+  client.setTimeout(
+      std::chrono::seconds(GetGlobalPluginProperties().GetTimeout()));
 
   llvm::StreamedHTTPResponseHandler Handler(
       [dest = tmp_file.str().str()]()
diff --git a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td
index 0cd631a80b90b..ef7ee107407d7 100644
--- a/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td
+++ b/lldb/source/Plugins/SymbolLocator/SymStore/SymbolLocatorSymStoreProperties.td
@@ -4,4 +4,7 @@ let Definition = "symbollocatorsymstore", Path = "plugin.symbol-locator.symstore
   def SymStoreURLs : Property<"urls", "Array">,
     ElementType<"String">,
     Desc<"List of local symstore directories to query for symbols">;
+  def Timeout : Property<"timeout", "UInt64">,
+    DefaultUnsignedValue<60>,
+    Desc<"Timeout in seconds for send/receive in HTTP connections to symbol servers.">;
 }
diff --git a/lldb/test/API/symstore/TestSymStore.py b/lldb/test/API/symstore/TestSymStore.py
index 5baa5fa4d0da8..012fbca25ff44 100644
--- a/lldb/test/API/symstore/TestSymStore.py
+++ b/lldb/test/API/symstore/TestSymStore.py
@@ -3,6 +3,8 @@
 import shutil
 import socketserver
 import threading
+import time
+import sys
 from functools import partial
 
 import lldb
@@ -90,6 +92,23 @@ def __exit__(self, *exc_info):
             self._thread.join()
 
 
+class SlowHTTPHandler(http.server.BaseHTTPRequestHandler):
+    """HTTP request handler that delays responses to simulate a slow server."""
+
+    delay = 2  # seconds to sleep before responding; set per test
+
+    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
+
+    def log_message(self, *args):
+        pass  # suppress server-side output
+
+
 class SymStoreTests(TestBase):
     SHARED_BUILD_TESTCASE = False
     TEST_WITH_PDB_DEBUG_INFO = True
@@ -169,3 +188,23 @@ def test_http(self):
             with HTTPServer(dir) as url:
                 self.runCmd(f"settings set plugin.symbol-locator.symstore.urls {url}")
                 self.try_breakpoint(exe, should_have_loc=True)
+
+    def test_http_timeout(self):
+        """
+        Check that a warning is emitted and no symbol is found when the server
+        takes longer to respond than the configured timeout.
+        """
+        exe, sym = self.build_inferior()
+        with MockedSymStore(self, exe, sym) as dir:
+            SlowHTTPHandler.delay = 3  # seconds; exceeds the 1s timeout below
+            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)

``````````

</details>


https://github.com/llvm/llvm-project/pull/192061