[Lldb-commits] [lldb] [lldb] Fix assert in ScriptedProcess destructor (PR #71744)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/71744

>From a26236eec419adc99c188f9fe3d0440f81aff0da Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <ismail at bennani.ma>
Date: Wed, 8 Nov 2023 16:24:57 -0800
Subject: [PATCH] [lldb] Fix assert in ScriptedProcess destructor

This patch should fix a test failure in `Expr/TestIRMemoryMapWindows.test`:

https://lab.llvm.org/buildbot/#/builders/219/builds/6786

The problem here is that since 7991412 landed, all the
`ScriptInterpreter::CreateScripted*Interface` now return a `nullptr`
when using the base `ScriptInterpreter` instance, instead of
`ScriptInterpreterPython` for instance.

This nullptr is actually well handled in the various places where we
create a Scripted Interface, however, because of the way to instanciate
a process, the process plugin manager have to iterate over every process
plugin and call the `CreateInstance` static function that should
instanciate the right object.

So in the ScriptedProcess case, because we are getting a `nullptr` when
trying to create a `ScriptedProcessInterface`, we try to discard the
process object, which calls the Process destructor, which in turns
calls the `ScriptedProcess` plugin `IsAlive` method. That method will
fire an assertion if the scripted interface pointer is not allocated.

This patch address that issue by setting a flag when destroying the
ScriptedProcess object, and checks that flag when calling `IsAlive`.

Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
---
 lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp | 6 +++++-
 lldb/source/Plugins/Process/scripted/ScriptedProcess.h   | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
index f2a647c1b0bea4c..7e48ea0fd2d935d 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -129,6 +129,8 @@ ScriptedProcess::ScriptedProcess(lldb::TargetSP target_sp,
 }
 
 ScriptedProcess::~ScriptedProcess() {
+  std::lock_guard<std::mutex> g(m_finalizing_mutex);
+  m_is_finalizing = true;
   Clear();
   // We need to call finalize on the process before destroying ourselves to
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
@@ -212,7 +214,9 @@ void ScriptedProcess::DidAttach(ArchSpec &process_arch) {
 
 Status ScriptedProcess::DoDestroy() { return Status(); }
 
-bool ScriptedProcess::IsAlive() { return GetInterface().IsAlive(); }
+bool ScriptedProcess::IsAlive() {
+  return !m_is_finalizing && GetInterface().IsAlive();
+}
 
 size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                                      Status &error) {
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
index 0335364b4010b22..42e1d67740b7483 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -129,6 +129,8 @@ class ScriptedProcess : public Process {
   // Member variables.
   const ScriptedMetadata m_scripted_metadata;
   lldb::ScriptedProcessInterfaceUP m_interface_up;
+  std::mutex m_finalizing_mutex;
+  bool m_is_finalizing = false;
 };
 
 } // namespace lldb_private