Fix an issue where certain CommandObjects (or Options thereof) were being created once, bound to a specific CommandInterpreter (and hence a specific Debugger), and then cached for reuse across different Debugger instances

Obviously, if the original Debugger goes away, those commands are holding on to now stale memory, which has the potential to cause crashes

Fixes rdar://24460882

llvm-svn: 259964

5 files changed, 79 insertions, 29 deletions

diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py b/lldb/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py
new file mode 100644
index 00000000000..a9b78069c3c
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py
@@ -0,0 +1,48 @@
+"""
+Test that commands do not try and hold on to stale CommandInterpreters in a multiple debuggers scenario
+"""
+
+from __future__ import print_function
+
+
+
+import os, time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class MultipleDebuggersCommandsTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @no_debug_info_test
+    def test_multipledebuggers_commands(self):
+        """Test that commands do not try and hold on to stale CommandInterpreters in a multiple debuggers scenario"""
+        source_init_files = False
+        magic_text = "The following built-in commands may relate to 'env'"
+        
+        debugger_1 = lldb.SBDebugger.Create(source_init_files)
+        interpreter_1 = debugger_1.GetCommandInterpreter()
+        
+        retobj = lldb.SBCommandReturnObject()
+        interpreter_1.HandleCommand("apropos env", retobj)
+        self.assertTrue(magic_text in str(retobj), "[interpreter_1]: the output does not contain the correct words")
+        
+        if self.TraceOn(): print(str(retobj))
+        
+        lldb.SBDebugger.Destroy(debugger_1)
+        
+        # now do this again with a different debugger - we shouldn't crash
+        
+        debugger_2 = lldb.SBDebugger.Create(source_init_files)
+        interpreter_2 = debugger_2.GetCommandInterpreter()
+        
+        retobj = lldb.SBCommandReturnObject()
+        interpreter_2.HandleCommand("apropos env", retobj)
+        self.assertTrue(magic_text in str(retobj), "[interpreter_2]: the output does not contain the correct words")
+        
+        if self.TraceOn(): print(str(retobj))
+        
+        lldb.SBDebugger.Destroy(debugger_2)
+        
diff --git a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
index 138a59aef68..362c5411a16 100644
--- a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -4167,12 +4167,7 @@ RenderScriptRuntime::RenderScriptRuntime(Process *process)
 lldb::CommandObjectSP
 RenderScriptRuntime::GetCommandObject(lldb_private::CommandInterpreter &interpreter)
 {
-    static CommandObjectSP command_object;
-    if (!command_object)
-    {
-        command_object.reset(new CommandObjectRenderScriptRuntime(interpreter));
-    }
-    return command_object;
+    return CommandObjectSP(new CommandObjectRenderScriptRuntime(interpreter));
 }
 
 RenderScriptRuntime::~RenderScriptRuntime() = default;
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index c7564655a11..3482b608bcc 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -41,6 +41,9 @@ using namespace lldb_private;
 //------------------------------------------------------------------
 PlatformPOSIX::PlatformPOSIX (bool is_host) :
 Platform(is_host),  // This is the local host platform
+m_option_group_platform_rsync(new OptionGroupPlatformRSync()),
+m_option_group_platform_ssh(new OptionGroupPlatformSSH()),
+m_option_group_platform_caching(new OptionGroupPlatformCaching()),
 m_remote_platform_sp ()
 {
 }
@@ -69,14 +72,17 @@ PlatformPOSIX::GetModuleSpec (const FileSpec& module_file_spec,
 lldb_private::OptionGroupOptions*
 PlatformPOSIX::GetConnectionOptions (lldb_private::CommandInterpreter& interpreter)
 {
-    if (m_options.get() == NULL)
+    auto iter = m_options.find(&interpreter), end = m_options.end();
+    if (iter == end)
     {
-        m_options.reset(new OptionGroupOptions(interpreter));
-        m_options->Append(new OptionGroupPlatformRSync());
-        m_options->Append(new OptionGroupPlatformSSH());
-        m_options->Append(new OptionGroupPlatformCaching());
+        std::unique_ptr<lldb_private::OptionGroupOptions> options(new OptionGroupOptions(interpreter));
+        options->Append(m_option_group_platform_rsync.get());
+        options->Append(m_option_group_platform_ssh.get());
+        options->Append(m_option_group_platform_caching.get());
+        m_options[&interpreter] = std::move(options);
     }
-    return m_options.get();
+    
+    return m_options.at(&interpreter).get();
 }
 
 bool
@@ -675,29 +681,21 @@ PlatformPOSIX::ConnectRemote (Args& args)
 
     if (error.Success() && m_remote_platform_sp)
     {
-        if (m_options.get())
+        if (m_option_group_platform_rsync.get() && m_option_group_platform_ssh.get() && m_option_group_platform_caching.get())
         {
-            OptionGroupOptions* options = m_options.get();
-            const OptionGroupPlatformRSync *m_rsync_options =
-                static_cast<const OptionGroupPlatformRSync *>(options->GetGroupWithOption('r'));
-            const OptionGroupPlatformSSH *m_ssh_options =
-                static_cast<const OptionGroupPlatformSSH *>(options->GetGroupWithOption('s'));
-            const OptionGroupPlatformCaching *m_cache_options =
-                static_cast<const OptionGroupPlatformCaching *>(options->GetGroupWithOption('c'));
-
-            if (m_rsync_options->m_rsync)
+            if (m_option_group_platform_rsync->m_rsync)
             {
                 SetSupportsRSync(true);
-                SetRSyncOpts(m_rsync_options->m_rsync_opts.c_str());
-                SetRSyncPrefix(m_rsync_options->m_rsync_prefix.c_str());
-                SetIgnoresRemoteHostname(m_rsync_options->m_ignores_remote_hostname);
+                SetRSyncOpts(m_option_group_platform_rsync->m_rsync_opts.c_str());
+                SetRSyncPrefix(m_option_group_platform_rsync->m_rsync_prefix.c_str());
+                SetIgnoresRemoteHostname(m_option_group_platform_rsync->m_ignores_remote_hostname);
             }
-            if (m_ssh_options->m_ssh)
+            if (m_option_group_platform_ssh->m_ssh)
             {
                 SetSupportsSSH(true);
-                SetSSHOpts(m_ssh_options->m_ssh_opts.c_str());
+                SetSSHOpts(m_option_group_platform_ssh->m_ssh_opts.c_str());
             }
-            SetLocalCacheDirectory(m_cache_options->m_cache_dir.c_str());
+            SetLocalCacheDirectory(m_option_group_platform_caching->m_cache_dir.c_str());
         }
     }
 
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h
index 60f6207d140..4f1f2200281 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -12,6 +12,7 @@
 
 // C Includes
 // C++ Includes
+#include <map>
 #include <memory>
 
 // Other libraries and framework includes
@@ -192,7 +193,11 @@ public:
     ConnectToWaitingProcesses(lldb_private::Debugger& debugger, lldb_private::Error& error) override;
 
 protected:
-    std::unique_ptr<lldb_private::OptionGroupOptions> m_options;
+    std::unique_ptr<lldb_private::OptionGroupPlatformRSync> m_option_group_platform_rsync;
+    std::unique_ptr<lldb_private::OptionGroupPlatformSSH> m_option_group_platform_ssh;
+    std::unique_ptr<lldb_private::OptionGroupPlatformCaching> m_option_group_platform_caching;
+    
+    std::map<lldb_private::CommandInterpreter*,std::unique_ptr<lldb_private::OptionGroupOptions>> m_options;
     lldb::PlatformSP m_remote_platform_sp; // Allow multiple ways to connect to a remote POSIX-compliant OS
 
     lldb_private::Error
diff --git a/lldb/source/Target/LanguageRuntime.cpp b/lldb/source/Target/LanguageRuntime.cpp
index b1e2b3eb04f..7f386162d16 100644
--- a/lldb/source/Target/LanguageRuntime.cpp
+++ b/lldb/source/Target/LanguageRuntime.cpp
@@ -336,6 +336,10 @@ LanguageRuntime::InitializeCommands (CommandObject* parent)
             CommandObjectSP command = command_callback(parent->GetCommandInterpreter());
             if (command)
             {
+                // the CommandObject vended by a Language plugin cannot be created once and cached because
+                // we may create multiple debuggers and need one instance of the command each - the implementing function
+                // is meant to create a new instance of the command each time it is invoked
+                assert(&command->GetCommandInterpreter() == &parent->GetCommandInterpreter() && "language plugin returned command for a mismatched CommandInterpreter");
                 parent->LoadSubCommand(command->GetCommandName(), command);
             }
         }