From a238f6cf6ebdc83fa55c4975739798e771808a7b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 00:24:26 +0000 Subject: [PATCH 1/4] Initial plan From 2c6edbe2145bd60859eae8047ddc61ff3e363f45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 08:53:18 +0000 Subject: [PATCH 2/4] Pass DJI to BindPatch to avoid deadlock-causing GetJitInfo call Modified BindPatch to accept an optional DebuggerJitInfo parameter. When DJI is provided by the caller, use it instead of calling GetJitInfo, which can trigger a deadlock through HashMap lookups with GC mode transitions. Updated two call sites that already have the DJI available: - AddBindAndActivatePatchForMethodDesc - MapPatchToDJI This is a targeted fix to avoid the deadlock described in issue where DebuggerController lock is held while calling GetJitInfo, which can try to acquire locks in the wrong order. Co-authored-by: noahfalk <6243776+noahfalk@users.noreply.github.com> --- src/coreclr/debug/ee/controller.cpp | 9 ++++++--- src/coreclr/debug/ee/controller.h | 3 ++- src/coreclr/debug/ee/debugger.cpp | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 14b14ff97e7b90..9496f1931c0bc0 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -1373,7 +1373,8 @@ void DebuggerController::Dequeue() // returns false, *pFail = true bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, MethodDesc *pMD, - CORDB_ADDRESS_TYPE *startAddr) + CORDB_ADDRESS_TYPE *startAddr, + DebuggerJitInfo *dji) { CONTRACTL { @@ -1415,7 +1416,9 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, _ASSERTE(!g_pEEInterface->IsStub((const BYTE *)startAddr)); // If we've jitted, map to a native offset. - DebuggerJitInfo *info = g_pDebugger->GetJitInfo(pMD, (const BYTE *)startAddr); + // If the caller provided a DJI, use it to avoid calling GetJitInfo which can trigger + // a deadlock-prone code path through HashMap lookups that do GC mode transitions. + DebuggerJitInfo *info = (dji != NULL) ? dji : g_pDebugger->GetJitInfo(pMD, (const BYTE *)startAddr); #ifdef LOGGING if (info == NULL) @@ -2261,7 +2264,7 @@ BOOL DebuggerController::AddBindAndActivatePatchForMethodDesc(MethodDesc *fd, 0, dji); - if (DebuggerController::BindPatch(patch, fd, NULL)) + if (DebuggerController::BindPatch(patch, fd, NULL, dji)) { DebuggerController::ActivatePatch(patch); ok = TRUE; diff --git a/src/coreclr/debug/ee/controller.h b/src/coreclr/debug/ee/controller.h index 9fc60b4f810dbb..b214d1f7f59d7d 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -1213,7 +1213,8 @@ class DebuggerController static bool BindPatch(DebuggerControllerPatch *patch, MethodDesc *fd, - CORDB_ADDRESS_TYPE *startAddr); + CORDB_ADDRESS_TYPE *startAddr, + DebuggerJitInfo *dji = NULL); static bool ApplyPatch(DebuggerControllerPatch *patch); static bool UnapplyPatch(DebuggerControllerPatch *patch); static bool IsPatched(CORDB_ADDRESS_TYPE *address, BOOL native); diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index bf74173a82bec9..4eab8c10506347 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -4960,7 +4960,7 @@ HRESULT Debugger::MapPatchToDJI(DebuggerControllerPatch *dcp, DebuggerJitInfo *d // We have an unbound native patch (eg. for PatchTrace), lets try to bind and activate it dcp->SetDJI(djiTo); LOG((LF_CORDB, LL_EVERYTHING, "D::MPTDJI trying to bind patch... could be problem\n")); - if (DebuggerController::BindPatch(dcp, djiTo->m_nativeCodeVersion.GetMethodDesc(), NULL)) + if (DebuggerController::BindPatch(dcp, djiTo->m_nativeCodeVersion.GetMethodDesc(), NULL, djiTo)) { DebuggerController::ActivatePatch(dcp); LOG((LF_CORDB, LL_INFO1000, "D::MPTDJI Binding went fine!\n" )); From 581a7bb0bb121579f02129d49d47824c1a8f5acc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 00:26:46 +0000 Subject: [PATCH 3/4] Use patch->GetDJI() instead of passing DJI as parameter Per code review feedback, simplified the fix to use the DJI already stored on the patch rather than passing it as a new parameter. This achieves the same goal of avoiding the deadlock-causing GetJitInfo call while being cleaner and more aligned with existing code patterns. Co-authored-by: AaronRobinsonMSFT <30635565+AaronRobinsonMSFT@users.noreply.github.com> --- src/coreclr/debug/ee/controller.cpp | 17 ++++++++++++----- src/coreclr/debug/ee/controller.h | 3 +-- src/coreclr/debug/ee/debugger.cpp | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 9496f1931c0bc0..125c4051c2a093 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -1373,8 +1373,7 @@ void DebuggerController::Dequeue() // returns false, *pFail = true bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, MethodDesc *pMD, - CORDB_ADDRESS_TYPE *startAddr, - DebuggerJitInfo *dji) + CORDB_ADDRESS_TYPE *startAddr) { CONTRACTL { @@ -1416,9 +1415,17 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, _ASSERTE(!g_pEEInterface->IsStub((const BYTE *)startAddr)); // If we've jitted, map to a native offset. - // If the caller provided a DJI, use it to avoid calling GetJitInfo which can trigger + // If the patch already has a DJI, use it to avoid calling GetJitInfo which can trigger // a deadlock-prone code path through HashMap lookups that do GC mode transitions. - DebuggerJitInfo *info = (dji != NULL) ? dji : g_pDebugger->GetJitInfo(pMD, (const BYTE *)startAddr); + DebuggerJitInfo *info = NULL; + if (patch->HasDJI()) + { + info = patch->GetDJI(); + } + else + { + info = g_pDebugger->GetJitInfo(pMD, (const BYTE *)startAddr); + } #ifdef LOGGING if (info == NULL) @@ -2264,7 +2271,7 @@ BOOL DebuggerController::AddBindAndActivatePatchForMethodDesc(MethodDesc *fd, 0, dji); - if (DebuggerController::BindPatch(patch, fd, NULL, dji)) + if (DebuggerController::BindPatch(patch, fd, NULL)) { DebuggerController::ActivatePatch(patch); ok = TRUE; diff --git a/src/coreclr/debug/ee/controller.h b/src/coreclr/debug/ee/controller.h index b214d1f7f59d7d..9fc60b4f810dbb 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -1213,8 +1213,7 @@ class DebuggerController static bool BindPatch(DebuggerControllerPatch *patch, MethodDesc *fd, - CORDB_ADDRESS_TYPE *startAddr, - DebuggerJitInfo *dji = NULL); + CORDB_ADDRESS_TYPE *startAddr); static bool ApplyPatch(DebuggerControllerPatch *patch); static bool UnapplyPatch(DebuggerControllerPatch *patch); static bool IsPatched(CORDB_ADDRESS_TYPE *address, BOOL native); diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 4eab8c10506347..bf74173a82bec9 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -4960,7 +4960,7 @@ HRESULT Debugger::MapPatchToDJI(DebuggerControllerPatch *dcp, DebuggerJitInfo *d // We have an unbound native patch (eg. for PatchTrace), lets try to bind and activate it dcp->SetDJI(djiTo); LOG((LF_CORDB, LL_EVERYTHING, "D::MPTDJI trying to bind patch... could be problem\n")); - if (DebuggerController::BindPatch(dcp, djiTo->m_nativeCodeVersion.GetMethodDesc(), NULL, djiTo)) + if (DebuggerController::BindPatch(dcp, djiTo->m_nativeCodeVersion.GetMethodDesc(), NULL)) { DebuggerController::ActivatePatch(dcp); LOG((LF_CORDB, LL_INFO1000, "D::MPTDJI Binding went fine!\n" )); From 6a6a4d62ff6ffd1c36b9d5c0eab2ce5ce9467b27 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 00:28:30 +0000 Subject: [PATCH 4/4] Simplify to use ternary operator for cleaner code Per code review feedback, use a ternary operator instead of if-else block for more concise and idiomatic C++ code. Co-authored-by: AaronRobinsonMSFT <30635565+AaronRobinsonMSFT@users.noreply.github.com> --- src/coreclr/debug/ee/controller.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 125c4051c2a093..c89a0918f14a42 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -1417,15 +1417,7 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, // If we've jitted, map to a native offset. // If the patch already has a DJI, use it to avoid calling GetJitInfo which can trigger // a deadlock-prone code path through HashMap lookups that do GC mode transitions. - DebuggerJitInfo *info = NULL; - if (patch->HasDJI()) - { - info = patch->GetDJI(); - } - else - { - info = g_pDebugger->GetJitInfo(pMD, (const BYTE *)startAddr); - } + DebuggerJitInfo *info = patch->HasDJI() ? patch->GetDJI() : g_pDebugger->GetJitInfo(pMD, (const BYTE *)startAddr); #ifdef LOGGING if (info == NULL)