From 283306c710b4671ce2fba539935290f2733fe255 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 f56860c600189b8b1ea3fdea89d17db9823bfe4d 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 0a6644c273e40a..35db62ed780db1 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -1379,7 +1379,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 { @@ -1421,7 +1422,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) @@ -2239,7 +2242,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 10bc8c8a26da50..8397e19a7ea789 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -1212,7 +1212,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 918be469bcc5a9..a8d6cff9db7b25 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -4984,7 +4984,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 03cb371dbc7a2a9c0b4f3b236c03050e16af1792 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 35db62ed780db1..d3533febe7bd93 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -1379,8 +1379,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 { @@ -1422,9 +1421,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) @@ -2242,7 +2249,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 8397e19a7ea789..10bc8c8a26da50 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -1212,8 +1212,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 a8d6cff9db7b25..918be469bcc5a9 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -4984,7 +4984,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 0cb7329ab2163a9d9a58bd65d0c80f6f1598a378 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 d3533febe7bd93..1bdb61cf378f46 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -1423,15 +1423,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)