[lldb] Add SBModule::GarbageCollectAllocatedModules and clear modules after each test run

[teemperor]

Summary

Right now the only places in the SB API where lldb:: ModuleSP instances are destroyed are in SBDebugger::MemoryPressureDetected
(where it's just attempted but not guaranteed) and in SBDebugger::DeleteTarget (which will be removed in D83933).
Tests that directly create an lldb::ModuleSP and never create a target therefore currently leak lldb::Module instances. This triggers the sanity checks in lldbtest that make sure
that the global module list is empty after a test.

This patch adds SBModule::GarbageCollectAllocatedModules as an explicit way to clean orphaned lldb::ModuleSP instances. Also we now
start calling this method at the end of each test run and move the sanity check behind that call to make this work. This way
even tests that don't create targets can pass the sanity check.

This fixes TestUnicodeSymbols.py when D83865 is applied (which makes that the sanity checks actually fail the test).

[labath]

I have no opinion on this. Jim may have one...

[teemperor]

I'm actually thinking that this maybe should go into the SBDebugger::Destroy functions. This way we also get all lldb::ModuleSP instances that are freed by the Debugger destructor.

[jingham]

We certainly don't want to clear the shared module cache when the Debugger or the SBDebugger is destroyed. Most IDE's that use LLDB as a library use a debugger per debugging session, and destroy the debugger when the debugging session ends. As it stands now, when the first debugging session ends we don't remove any of the already parsed modules, so that the second debugging session is much faster. That would not be true if you did RemoveOrphanedSharedModules in Debugger::Destroy. 99% of all the libraries that get loaded by an app don't change over an Xcode run, and are in fact shared among most of the targets you might debug on a given platform. So clearing the cache after every run would result in a lot of pointless reparsing of system libraries.

The choice to clear the shared module cache should always be left up to the client.

[teemperor]

In D83876#2153709, @jingham wrote:

We certainly don't want to clear the shared module cache when the Debugger or the SBDebugger is destroyed. Most IDE's that use LLDB as a library use a debugger per debugging session, and destroy the debugger when the debugging session ends. As it stands now, when the first debugging session ends we don't remove any of the already parsed modules, so that the second debugging session is much faster. That would not be true if you did RemoveOrphanedSharedModules in Debugger::Destroy. 99% of all the libraries that get loaded by an app don't change over an Xcode run, and are in fact shared among most of the targets you might debug on a given platform. So clearing the cache after every run would result in a lot of pointless reparsing of system libraries.

The choice to clear the shared module cache should always be left up to the client.

So I guess that only leaves an explicit function for that as an option.

Also, if that's the concept then we should also delete the same code in DeleteTarget. I'll make a review for that.

[teemperor]

• Moved to ClearCachedModules

[JDevlieghere]

lldb/bindings/interface/SBModule.i

365

Maybe we should call this ClearAllocatedModules for consistency with the other APIs?

[teemperor]

lldb/bindings/interface/SBModule.i

365

Not sure what's a good name. I would assume from the name ClearAllocatedModules that i would delete all allocated modules, but it actually only clears modules that are only kept around for the sake of caching. So this would not actually always work (but it should in the test suite):

assert(GetNumberAllocatedModules != 0);
ClearAllocatedModules();
assert(GetNumberAllocatedModules == 0); // boom if any of the allocated modules is still needed.

(FWIW, LLDB internally calls this "orphaned" modules but that doesn't seem very intuitive either).

"MaybeClearAllocatedModules", "TryClearAllocatedModules", "ClearOrphanedModules", etc.

[JDevlieghere]

lldb/bindings/interface/SBModule.i

365

How about GarbageCollectAllocatedModules?

[teemperor]

lldb/bindings/interface/SBModule.i

365

I actually like this one, I'll update the patch!

[teemperor]

• Rename to GarbageCollectAllocatedModules

lldb/bindings/interface/SBModule.i

public:
     Returns the number of modules in the module cache. This is an
     implementation detail exposed for testing and should not be relied upon.
 
     @return
         The number of modules in the module cache.") GetNumberAllocatedModules;
     static uint32_t
     GetNumberAllocatedModules();
 
+    %feature("docstring", "
+    Removes all modules which are no longer needed by any part of LLDB from
+    the module cache.
+
+    This is an implementation detail exposed for testing and should not be
+    relied upon. Use SBDebugger::MemoryPressureDetected instead to reduce
+    LLDB's memory consumption during execution.
+    ") GarbageCollectAllocatedModules;
+    static void
+    GarbageCollectAllocatedModules();
+
     STRING_EXTENSION(SBModule)
 
 #ifdef SWIGPYTHON
     %pythoncode %{
         def __len__(self):
             '''Return the number of symbols in a lldb.SBModule object.'''
             return self.GetNumSymbols()
 

lldb/include/lldb/API/SBModule.h

public:
   lldb::SBFileSpec GetSymbolFileSpec() const;
 
   lldb::SBAddress GetObjectFileHeaderAddress() const;
   lldb::SBAddress GetObjectFileEntryPointAddress() const;
 
   /// Get the number of global modules.
   static uint32_t GetNumberAllocatedModules();
 
+  /// Remove any global modules which are no longer needed.
+  static void GarbageCollectAllocatedModules();
+
 private:
   friend class SBAddress;
   friend class SBFrame;
   friend class SBSection;
   friend class SBSymbolContext;
   friend class SBTarget;
 
   explicit SBModule(const lldb::ModuleSP &module_sp);

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

def tearDown(self):
         # Remove subprocesses created by the test.
         self.cleanupSubprocesses()
 
         # This must be the last statement, otherwise teardown hooks or other
         # lines might depend on this still being active.
         lldb.SBDebugger.Destroy(self.dbg)
         del self.dbg
 
+        # All modules should be orphaned now so that they can be cleared from
+        # the shared module cache.
+        lldb.SBModule.GarbageCollectAllocatedModules()
+
+        # Modules are not orphaned during reproducer replay because they're
+        # leaked on purpose.
+        if not configuration.is_reproducer():
+            # Assert that the global module cache is empty.
+            self.assertEqual(lldb.SBModule.GetNumberAllocatedModules(), 0)
+
+
     # =========================================================
     # Various callbacks to allow introspection of test progress
     # =========================================================
 
     def markError(self):
         """Callback invoked when an error (unexpected exception) errored."""
         self.__errored__ = True
         with recording(self, False) as sbuf:
def tearDown(self):
                 targets.append(target)
                 process = target.GetProcess()
                 if process:
                     rc = self.invoke(process, "Kill")
                     assert rc.Success()
         for target in targets:
             self.dbg.DeleteTarget(target)
 
-        # Modules are not orphaned during reproducer replay because they're
-        # leaked on purpose.
         if not configuration.is_reproducer():
             # Assert that all targets are deleted.
-            assert self.dbg.GetNumTargets() == 0
-            # Assert that the global module cache is empty.
-            assert lldb.SBModule.GetNumberAllocatedModules() == 0
+            self.assertEqual(self.dbg.GetNumTargets(), 0)
 
         # Do this last, to make sure it's in reverse order from how we setup.
         Base.tearDown(self)
 
     def switch_to_thread_with_stop_reason(self, stop_reason):
         """
         Run the 'thread list' command, and select the thread with stop reason as
         'stop_reason'.  If no such thread exists, no select action is done.

lldb/source/API/SBModule.cpp

 
 uint32_t SBModule::GetNumberAllocatedModules() {
   LLDB_RECORD_STATIC_METHOD_NO_ARGS(uint32_t, SBModule,
                                     GetNumberAllocatedModules);
 
   return Module::GetNumberAllocatedModules();
 }
 
+void SBModule::GarbageCollectAllocatedModules() {
+  LLDB_RECORD_STATIC_METHOD_NO_ARGS(void, SBModule,
+                                    GarbageCollectAllocatedModules);
+  const bool mandatory = false;
+  ModuleList::RemoveOrphanSharedModules(mandatory);
+}
+
 namespace lldb_private {
 namespace repro {
 
 template <>
 void RegisterMethods<SBModule>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBModule, ());
   LLDB_REGISTER_CONSTRUCTOR(SBModule, (const lldb::SBModuleSpec &));
   LLDB_REGISTER_CONSTRUCTOR(SBModule, (const lldb::SBModule &));