Increase testsuite packet-timeout 5secs -> 1min

[jankratochvil]

Summary

rL357954 did increase packet-timeout 1sec -> 5secs. Which is IMO about the maximum timeout reasonable for regular use. But for testsuite I think the timeout should be higher as the testsuite runs in parallel and it can be run even on slow hosts and with other load (moreover if it runs on some slow arch).
I have chosen 60 secs, that should be enough hopefully. Larger value could make debugging with hanging lldb-server annoying.
This patch was based on this testsuite timeout:
http://lab.llvm.org:8014/builders/lldb-x86_64-fedora/builds/546/steps/test/logs/stdio

FAIL: test_connect (TestGDBRemoteClient.TestGDBRemoteClient)
   Test connecting to a remote gdb server
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py", line 13, in test_connect
    process = self.connect(target)
  File "/home/jkratoch/slave-lldb-x86_64-fedora/lldb-x86_64-fedora/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py", line 480, in connect
    self.assertTrue(error.Success(), error.description)
AssertionError: False is not True : failed to get reply to handshake packet

[jankratochvil]

lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py

24

Here it was always timing out (and still reporting PASS) which was not a problem for 5 seconds timeout but with 5 minutes timeout it was hanging during the testsuite run.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

214 ↗

(On Diff #211712)

The failed TestGDBRemoteClient testcases did timeout despite this increase to 6 seconds (which made more sense before rL357954 when the default timeout was just 1 second and not the current 5 seconds).
If this patch is not welcome maybe we could try running the testsuite just increasing this seconds(6) to some seconds(60).

lldb/tools/lldb-test/lldb-test.cpp

984

I somehow failed to find how to access the timeout by API, I can try again.

[labath]

Why not unify now? It shouldn't be too hard to at least unify all the instances in dotest python code to a single place. That would already cut the number of places that need to be kept synced in half...

lldb/tools/lldb-test/lldb-test.cpp

984

I wouldn't be surprised if there is no api for that.

[jingham]

There isn't an SB API for "settings set" yet. I've put off doing that because the "settings" subsystem is currently unfinished. In the original design you would be specify qualifiers in the setting hierarchy, like:

settings set target[arch=x86_64].whatever

That would be pretty handy in a lot of cases - for instance there are some settings that you have to set when doing KEXT development on macOS that are not appropriate for user-space debugging and this trips up the kernel developers all the time.

But we've never gotten around to designing this fully, and I've don't want to design the SB API's for settings till we know how this is going to work. Apparently I didn't put this on the Projects page, I'll go rectify that in a bit.

[jankratochvil]

In D65271#1600829, @labath wrote:

Why not unify now? It shouldn't be too hard to at least unify all the instances in dotest python code to a single place. That would already cut the number of places that need to be kept synced in half...

Done; but I do not like much that from lldbsuite.test.lldbtest import Base in vscode.py but then other choices (passing Base.setUpCommands() from callers) looks even more messy to me.

[labath]

In D65271#1601717, @jankratochvil wrote:

In D65271#1600829, @labath wrote:

Why not unify now? It shouldn't be too hard to at least unify all the instances in dotest python code to a single place. That would already cut the number of places that need to be kept synced in half...

Done; but I do not like much that from lldbsuite.test.lldbtest import Base in vscode.py but then other choices (passing Base.setUpCommands() from callers) looks even more messy to me.

I think I'd go for threading it in from VSCodeTestCaseBase. Maybe not as an argument to request_attach and friends, but possibly as an argument to the DebugCommunication object constructor.

[jankratochvil]

In D65271#1602143, @labath wrote:

I think I'd go for threading it in from VSCodeTestCaseBase. Maybe not as an argument to request_attach and friends, but possibly as an argument to the DebugCommunication object constructor.

That looks as a good place to me, thanks. Be careful with the review I do not know Python almost at all.

[labath]

LGTM, thanks.

BTW, I was pretty much a python noob too when starting with lldb, but our test suite has forced me to become somewhat of an expert. :)

lldb/tools/lldb-test/lldb-test.cpp

979

lldb-test is not confined by the SB API. The reason this (unlike the symbols.enable-external-lookup thingy) does not work is because ProcessGDBRemote is a plugin, and so the API to access the setting is hidden by the plugin encapsulation...

[jankratochvil]

lldb/tools/lldb-test/lldb-test.cpp

979

Thanks for the explanation; still SB API could provide some function to lookup the plugin by its name to access its settings etc. I have rather deleted the comment there.

lldb/lit/lit-lldb-init.in

 # LLDB init file for the LIT tests.
 settings set symbols.enable-external-lookup false
+settings set plugin.process.gdb-remote.packet-timeout 60
 settings set interpreter.echo-comment-commands false

lldb/packages/Python/lldbsuite/test/api/multithreaded/driver.cpp.template

 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
 #endif
   int code = 0;
 
   SBDebugger::Initialize();
   SBDebugger dbg = SBDebugger::Create();
   dbg.HandleCommand("settings set symbols.enable-external-lookup false");
+  dbg.HandleCommand(
+      "settings set plugin.process.gdb-remote.packet-timeout 60");
 
   try {
     if (!dbg.IsValid())
       throw Exception("invalid debugger");
     vector<string> args(argv + 1, argv + argc);
 
     test(dbg, args);
   } catch (Exception &e) {
     cout << "ERROR: " << e.what() << endl;
     code = 1;
   }
 
   SBDebugger::Destroy(dbg);
   return code;
 }

lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoWatchpointSupportInfo.py

def test(self):
         class MyResponder(MockGDBServerResponder):
 
             def haltReason(self):
                 return "T02thread:1ff0d;thread-pcs:10001bc00;"
 
             def threadStopInfo(self, threadnum):
                 if threadnum == 0x1ff0d:
                     return "T02thread:1ff0d;thread-pcs:10001bc00;"
+                return ""
 
             def setBreakpoint(self, packet):
                 if packet.startswith("Z2,"):
                     return "OK"
 
             def qXferRead(self, obj, annex, offset, length):
                 if annex == "target.xml":
                     return """<?xml version="1.0"?>

lldb/packages/Python/lldbsuite/test/lldbtest.py

class Base(unittest2.TestCase):
     def getBuildArtifact(self, name="a.out"):
         """Return absolute path to an artifact in the test's build directory."""
         return os.path.join(self.getBuildDir(), name)
 
     def getSourcePath(self, name):
         """Return absolute path to a file in the test's source directory."""
         return os.path.join(self.getSourceDir(), name)
 
+    @staticmethod
+    def setUpCommands():
+        return [
+            # Disable Spotlight lookup. The testsuite creates
+            # different binaries with the same UUID, because they only
+            # differ in the debug info, which is not being hashed.
+            "settings set symbols.enable-external-lookup false",
+
+            # Testsuite runs in parallel and the host can have also other load.
+            "settings set plugin.process.gdb-remote.packet-timeout 60"]
+
     def setUp(self):
         """Fixture for unittest test case setup.
 
         It works with the test driver to conditionally skip tests and does other
         initializations."""
         #import traceback
         # traceback.print_stack()
 
         if "LIBCXX_PATH" in os.environ:
             self.libcxxPath = os.environ["LIBCXX_PATH"]
         else:
             self.libcxxPath = None
 
         if "LLDBVSCODE_EXEC" in os.environ:
             self.lldbVSCodeExec = os.environ["LLDBVSCODE_EXEC"]
         else:
             self.lldbVSCodeExec = None
 
-        self.lldbOption = "-o 'settings set symbols.enable-external-lookup false'"
+        self.lldbOption = " ".join(
+            "-o '" + s + "'" for s in self.setUpCommands())
 
         # If we spawn an lldb process for test (via pexpect), do not load the
         # init file unless told otherwise.
         if os.environ.get("NO_LLDBINIT") != "NO":
             self.lldbOption += " --no-lldbinit"
 
         # Assign the test method name to self.testMethodName.
         #
def setUp(self):
         # decorators.
         Base.setUp(self)
 
         # Set the clang modules cache path used by LLDB.
         mod_cache = os.path.join(os.environ["LLDB_BUILD"], "module-cache-lldb")
         self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
                     % mod_cache)
 
-        # Disable Spotlight lookup. The testsuite creates
-        # different binaries with the same UUID, because they only
-        # differ in the debug info, which is not being hashed.
-        self.runCmd('settings set symbols.enable-external-lookup false')
+        for s in self.setUpCommands():
+            self.runCmd(s)
 
         # Disable color.
         self.runCmd("settings set use-color false")
 
         # Make sure that a sanitizer LLDB's environment doesn't get passed on.
         if 'DYLD_LIBRARY_PATH' in os.environ:
             self.runCmd('settings set target.env-vars DYLD_LIBRARY_PATH=')
 

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

 from __future__ import print_function
 
 from lldbsuite.test.lldbtest import *
 import os
 import vscode
 
 
 class VSCodeTestCaseBase(TestBase):
 
     def create_debug_adaptor(self):
         '''Create the Visual Studio Code debug adaptor'''
         self.assertTrue(os.path.exists(self.lldbVSCodeExec),
                         'lldb-vscode must exist')
-        self.vscode = vscode.DebugAdaptor(executable=self.lldbVSCodeExec)
+        self.vscode = vscode.DebugAdaptor(
+            executable=self.lldbVSCodeExec, init_commands=Base.setUpCommands())
 
     def build_and_create_debug_adaptor(self):
         self.build()
         self.create_debug_adaptor()
 
     def set_source_breakpoints(self, source_path, lines, condition=None,
                                hitCondition=None):
         '''Sets source breakpoints and returns an array of strings containing

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

while not done:
         # `packet` will be `None` on EOF. We want to pass it down to
         # handle_recv_packet anyway so the main thread can handle unexpected
         # termination of lldb-vscode and stop waiting for new packets.
         done = not vs_comm.handle_recv_packet(packet)
 
 
 class DebugCommunication(object):
 
-    def __init__(self, recv, send):
+    def __init__(self, recv, send, init_commands):
         self.trace_file = None
         self.send = send
         self.recv = recv
         self.recv_packets = []
         self.recv_condition = threading.Condition()
         self.recv_thread = threading.Thread(target=read_packet_thread,
                                             args=(self,))
         self.process_event_body = None
         self.exit_status = None
         self.initialize_body = None
         self.thread_stop_reasons = {}
         self.sequence = 1
         self.threads = None
         self.recv_thread.start()
         self.output_condition = threading.Condition()
         self.output = {}
         self.configuration_done_sent = False
         self.frame_scopes = {}
+        self.init_commands = init_commands
 
     @classmethod
     def encode_content(cls, s):
         return ("Content-Length: %u\r\n\r\n%s" % (len(s), s)).encode("utf-8")
 
     @classmethod
     def validate_response(cls, command, response):
         if command['command'] != response['command']:
def request_attach(self, program=None, pid=None, waitFor=None, trace=None,
         if pid is not None:
             args_dict['pid'] = pid
         if program is not None:
             args_dict['program'] = program
         if waitFor is not None:
             args_dict['waitFor'] = waitFor
         if trace:
             args_dict['trace'] = trace
-        args_dict['initCommands'] = [
-            'settings set symbols.enable-external-lookup false']
+        args_dict['initCommands'] = self.init_commands
         if initCommands:
             args_dict['initCommands'].extend(initCommands)
         if preRunCommands:
             args_dict['preRunCommands'] = preRunCommands
         if stopCommands:
             args_dict['stopCommands'] = stopCommands
         if exitCommands:
             args_dict['exitCommands'] = exitCommands
def request_launch(self, program, args=None, cwd=None, env=None,
         if disableASLR:
             args_dict['disableASLR'] = disableASLR
         if disableSTDIO:
             args_dict['disableSTDIO'] = disableSTDIO
         if shellExpandArguments:
             args_dict['shellExpandArguments'] = shellExpandArguments
         if trace:
             args_dict['trace'] = trace
-        args_dict['initCommands'] = [
-            'settings set symbols.enable-external-lookup false']
+        args_dict['initCommands'] = self.init_commands
         if initCommands:
             args_dict['initCommands'].extend(initCommands)
         if preRunCommands:
             args_dict['preRunCommands'] = preRunCommands
         if stopCommands:
             args_dict['stopCommands'] = stopCommands
         if exitCommands:
             args_dict['exitCommands'] = exitCommands
def request_testGetTargetBreakpoints(self):
         return self.send_recv(command_dict)
 
     def terminate(self):
         self.send.close()
         # self.recv.close()
 
 
 class DebugAdaptor(DebugCommunication):
-    def __init__(self, executable=None, port=None):
+    def __init__(self, executable=None, port=None, init_commands=[]):
         self.process = None
         if executable is not None:
             self.process = subprocess.Popen([executable],
                                             stdin=subprocess.PIPE,
                                             stdout=subprocess.PIPE,
                                             stderr=subprocess.PIPE)
             DebugCommunication.__init__(self, self.process.stdout,
-                                        self.process.stdin)
+                                        self.process.stdin, init_commands)
         elif port is not None:
             s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
             s.connect(('127.0.0.1', port))
-            DebugCommunication.__init__(self, s.makefile('r'), s.makefile('w'))
+            DebugCommunication.__init__(self, s.makefile('r'), s.makefile('w'),
+                init_commands)
 
     def get_pid(self):
         if self.process:
             return self.process.pid
         return -1
 
     def terminate(self):
         super(DebugAdaptor, self).terminate()

lldb/tools/lldb-test/lldb-test.cpp

WithColor::error() << "initialization failed: " << toString(std::move(e))
                        << '\n';
     return 1;
   }
 
   CleanUp TerminateDebugger([&] { DebuggerLifetime.Terminate(); });
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
+  // There is no SB API for settings in general like:
+  // GetGlobalPluginProperties()->SetPacketTimeout(std::chrono::seconds(60));
+  CommandReturnObject Result;
+  Dbg->GetCommandInterpreter().HandleCommand(
+      "settings set plugin.process.gdb-remote.packet-timeout 60",
+      /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
     Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, errs());
 
   if (opts::BreakpointSubcommand)
     return opts::breakpoint::evaluateBreakpoints(*Dbg);
   if (opts::ObjectFileSubcommand)
     return dumpObjectFiles(*Dbg);
   if (opts::SymbolsSubcommand)
     return opts::symbols::dumpSymbols(*Dbg);
   if (opts::IRMemoryMapSubcommand)
     return opts::irmemorymap::evaluateMemoryMapCommands(*Dbg);
 
   WithColor::error() << "No command specified.\n";
   return 1;
 }