Samsung/netcoredbg

Incorrect thread identified as "Main Thread" in --attach mode

qgindi opened this issue · 2 comments

qgindi commented

In --attach mode the info threads command gets a list of threads where a random thread is labeled "Main Thread".

Another bad thing: also it overwrites Thread.Name. Therefore, even if I detect the true main thread, I cannot get a list with correct thread names.

Tested with --interpreter=cli and --interpreter=vscode, both the same.

Thanks, will be fixed at next sync. Looks like at attach, runtime could calls CreateThread callback with any sequence.
Here is draft patch:

From cb2e450b3a95e4e5e9f9ec1f71530cd6daff45c5 Mon Sep 17 00:00:00 2001
From: Mikhail Kurinnoi <m.kurinnoi@samsung.com>
Date: Thu, 23 Nov 2023 12:50:06 +0300
Subject: [PATCH] Remove main thread detection at attach.

---
 src/debugger/managedcallback.cpp |  2 +-
 src/debugger/threads.cpp         | 12 ++++++------
 src/debugger/threads.h           |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/debugger/managedcallback.cpp b/src/debugger/managedcallback.cpp
index 1f01533..c48d3dc 100644
--- a/src/debugger/managedcallback.cpp
+++ b/src/debugger/managedcallback.cpp
@@ -259,7 +259,7 @@ HRESULT STDMETHODCALLTYPE ManagedCallback::CreateThread(ICorDebugAppDomain *pApp
         LOGW("Thread was created by user code during evaluation with implicit user code execution.");
 
     ThreadId threadId(getThreadId(pThread));
-    m_debugger.m_sharedThreads->Add(threadId);
+    m_debugger.m_sharedThreads->Add(threadId, m_debugger.m_startMethod == StartAttach);
 
     m_debugger.pProtocol->EmitThreadEvent(ThreadEvent(ManagedThreadStarted, threadId, m_debugger.m_interopDebugging));
     return m_sharedCallbacksQueue->ContinueAppDomain(pAppDomain);
diff --git a/src/debugger/threads.cpp b/src/debugger/threads.cpp
index 20b094b..2d2e8ed 100644
--- a/src/debugger/threads.cpp
+++ b/src/debugger/threads.cpp
@@ -20,13 +20,13 @@ ThreadId getThreadId(ICorDebugThread *pThread)
     return SUCCEEDED(res) && threadId != 0 ? ThreadId{threadId} : ThreadId{};
 }
 
-void Threads::Add(const ThreadId &threadId)
+void Threads::Add(const ThreadId &threadId, bool processAttached)
 {
     std::unique_lock<Utility::RWLock::Writer> write_lock(m_userThreadsRWLock.writer);
 
     m_userThreads.emplace(threadId);
-    // First added user thread is Main thread for sure.
-    if (!MainThread)
+    // First added user thread during start is Main thread for sure.
+    if (!processAttached && !MainThread)
         MainThread = threadId;
 }
 
@@ -43,9 +43,6 @@ void Threads::Remove(const ThreadId &threadId)
 
 std::string Threads::GetThreadName(ICorDebugProcess *pProcess, const ThreadId &userThread)
 {
-    if (MainThread == userThread)
-        return "Main Thread";
-
     std::string threadName = "<No name>";
 
     if (m_sharedEvaluator)
@@ -82,6 +79,9 @@ std::string Threads::GetThreadName(ICorDebugProcess *pProcess, const ThreadId &u
         }
     }
 
+    if (MainThread && MainThread == userThread && threadName == "<No name>")
+        return "Main Thread";
+
     return threadName;
 }
 
diff --git a/src/debugger/threads.h b/src/debugger/threads.h
index a091b3c..5329669 100644
--- a/src/debugger/threads.h
+++ b/src/debugger/threads.h
@@ -32,7 +32,7 @@ class Threads
 
 public:
 
-    void Add(const ThreadId &threadId);
+    void Add(const ThreadId &threadId, bool processAttached);
     void Remove(const ThreadId &threadId);
     HRESULT GetThreadsWithState(ICorDebugProcess *pProcess, std::vector<Thread> &threads);
 #ifdef INTEROP_DEBUGGING
-- 
2.25.1

Fixed in upstream.