From 9736509c46ffbd9dca068756264881897550a80c Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 3 Nov 2025 17:31:20 +0100 Subject: [PATCH 1/7] gc hole + nested FailFast --- src/coreclr/classlibnative/bcltype/system.cpp | 6 +++++- src/coreclr/vm/excep.cpp | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/classlibnative/bcltype/system.cpp b/src/coreclr/classlibnative/bcltype/system.cpp index 30b215f6c9d344..91fea660395040 100644 --- a/src/coreclr/classlibnative/bcltype/system.cpp +++ b/src/coreclr/classlibnative/bcltype/system.cpp @@ -117,6 +117,8 @@ static StackWalkAction FindFailFastCallerCallback(CrawlFrame* frame, VOID* data) return SWA_ABORT; } +static int alreadyfailing = 0; + extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, PCWSTR message, QCall::ObjectHandleOnStack exception, PCWSTR errorSource) { QCALL_CONTRACT; @@ -125,6 +127,8 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, GCX_COOP(); + alreadyfailing++; + FindFailFastCallerStruct findCallerData; findCallerData.pStackMark = mark; findCallerData.retAddress = 0; @@ -143,7 +147,7 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, LPCWSTR argExceptionString = NULL; StackSString msg; - if (exception.Get() != NULL) + if (alreadyfailing == 1 && exception.Get() != NULL) { GetExceptionMessage(exception.Get(), msg); argExceptionString = msg.GetUnicode(); diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index d285f2b1f184d4..67577f7d03f27e 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -224,9 +224,11 @@ void GetExceptionMessage(OBJECTREF throwable, SString &result) STRINGREF pString = GetExceptionMessage(throwable); + GCPROTECT_BEGIN(pString); // If call returned NULL (not empty), oh well, no message. if (pString != NULL) pString->GetSString(result); + GCPROTECT_END(); } // void GetExceptionMessage() STRINGREF GetExceptionMessage(OBJECTREF throwable) From 95e4ce1111f12357a45595817ed58514955c15f6 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 3 Nov 2025 17:46:09 +0100 Subject: [PATCH 2/7] better comment --- src/coreclr/classlibnative/bcltype/system.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/classlibnative/bcltype/system.cpp b/src/coreclr/classlibnative/bcltype/system.cpp index 91fea660395040..246eab4f42e649 100644 --- a/src/coreclr/classlibnative/bcltype/system.cpp +++ b/src/coreclr/classlibnative/bcltype/system.cpp @@ -117,7 +117,7 @@ static StackWalkAction FindFailFastCallerCallback(CrawlFrame* frame, VOID* data) return SWA_ABORT; } -static int alreadyfailing = 0; +static int alreadyFailing = 0; extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, PCWSTR message, QCall::ObjectHandleOnStack exception, PCWSTR errorSource) { @@ -127,8 +127,6 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, GCX_COOP(); - alreadyfailing++; - FindFailFastCallerStruct findCallerData; findCallerData.pStackMark = mark; findCallerData.retAddress = 0; @@ -147,7 +145,10 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, LPCWSTR argExceptionString = NULL; StackSString msg; - if (alreadyfailing == 1 && exception.Get() != NULL) + // because Environment_FailFast should kill the process, any subsequent calls are likely nested call from managed while unwinding the stack. + // only collect exception string if this is the first attempt to fail fast. + alreadyFailing++; + if (alreadyFailing == 1 && exception.Get() != NULL) { GetExceptionMessage(exception.Get(), msg); argExceptionString = msg.GetUnicode(); From 7d2f47cae3eb58628c015f3f57d584af707e305f Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 3 Nov 2025 17:47:52 +0100 Subject: [PATCH 3/7] feedback --- src/coreclr/vm/excep.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 67577f7d03f27e..d285f2b1f184d4 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -224,11 +224,9 @@ void GetExceptionMessage(OBJECTREF throwable, SString &result) STRINGREF pString = GetExceptionMessage(throwable); - GCPROTECT_BEGIN(pString); // If call returned NULL (not empty), oh well, no message. if (pString != NULL) pString->GetSString(result); - GCPROTECT_END(); } // void GetExceptionMessage() STRINGREF GetExceptionMessage(OBJECTREF throwable) From 7a1930238b703ea4fe3b2e5d2928bbc8fe016857 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Tue, 4 Nov 2025 14:15:02 +0100 Subject: [PATCH 4/7] feedback --- src/coreclr/classlibnative/bcltype/system.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/classlibnative/bcltype/system.cpp b/src/coreclr/classlibnative/bcltype/system.cpp index 246eab4f42e649..2fcef267f1b4af 100644 --- a/src/coreclr/classlibnative/bcltype/system.cpp +++ b/src/coreclr/classlibnative/bcltype/system.cpp @@ -117,7 +117,7 @@ static StackWalkAction FindFailFastCallerCallback(CrawlFrame* frame, VOID* data) return SWA_ABORT; } -static int alreadyFailing = 0; +static thread_local int8_t alreadyFailing = 0; extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, PCWSTR message, QCall::ObjectHandleOnStack exception, PCWSTR errorSource) { @@ -148,7 +148,11 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, // because Environment_FailFast should kill the process, any subsequent calls are likely nested call from managed while unwinding the stack. // only collect exception string if this is the first attempt to fail fast. alreadyFailing++; - if (alreadyFailing == 1 && exception.Get() != NULL) + if (alreadyFailing != 1) + { + argExceptionString = u"Environment.FailFast called recursively."; + } + else if (exception.Get() != NULL) { GetExceptionMessage(exception.Get(), msg); argExceptionString = msg.GetUnicode(); From 00124b321906cff9761148ec21e29f6feb9f6859 Mon Sep 17 00:00:00 2001 From: Pavel Savara Date: Tue, 4 Nov 2025 14:39:59 +0100 Subject: [PATCH 5/7] Update src/coreclr/classlibnative/bcltype/system.cpp Co-authored-by: Jan Kotas --- src/coreclr/classlibnative/bcltype/system.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/classlibnative/bcltype/system.cpp b/src/coreclr/classlibnative/bcltype/system.cpp index 2fcef267f1b4af..5976255d4930f3 100644 --- a/src/coreclr/classlibnative/bcltype/system.cpp +++ b/src/coreclr/classlibnative/bcltype/system.cpp @@ -150,7 +150,7 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, alreadyFailing++; if (alreadyFailing != 1) { - argExceptionString = u"Environment.FailFast called recursively."; + argExceptionString = W("Environment.FailFast called recursively."); } else if (exception.Get() != NULL) { From 89bb278d02cb6c3f01270eae6e8e2c6502f894b6 Mon Sep 17 00:00:00 2001 From: Pavel Savara Date: Tue, 4 Nov 2025 14:40:06 +0100 Subject: [PATCH 6/7] Update src/coreclr/classlibnative/bcltype/system.cpp Co-authored-by: Jan Kotas --- src/coreclr/classlibnative/bcltype/system.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/classlibnative/bcltype/system.cpp b/src/coreclr/classlibnative/bcltype/system.cpp index 5976255d4930f3..b16e738f920c9b 100644 --- a/src/coreclr/classlibnative/bcltype/system.cpp +++ b/src/coreclr/classlibnative/bcltype/system.cpp @@ -145,8 +145,8 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, LPCWSTR argExceptionString = NULL; StackSString msg; - // because Environment_FailFast should kill the process, any subsequent calls are likely nested call from managed while unwinding the stack. - // only collect exception string if this is the first attempt to fail fast. + // Because Environment_FailFast should kill the process, any subsequent calls are likely nested call from managed while formatting the exception message or the stack trace. + // only collect exception string if this is the first attempt to fail fast on this thread. alreadyFailing++; if (alreadyFailing != 1) { From 4a42e4ecccd1a54a57b261c9b4a66b5fb84a591a Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 4 Nov 2025 05:42:16 -0800 Subject: [PATCH 7/7] Update src/coreclr/classlibnative/bcltype/system.cpp --- src/coreclr/classlibnative/bcltype/system.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/classlibnative/bcltype/system.cpp b/src/coreclr/classlibnative/bcltype/system.cpp index b16e738f920c9b..3790754ca7787e 100644 --- a/src/coreclr/classlibnative/bcltype/system.cpp +++ b/src/coreclr/classlibnative/bcltype/system.cpp @@ -146,7 +146,7 @@ extern "C" void QCALLTYPE Environment_FailFast(QCall::StackCrawlMarkHandle mark, LPCWSTR argExceptionString = NULL; StackSString msg; // Because Environment_FailFast should kill the process, any subsequent calls are likely nested call from managed while formatting the exception message or the stack trace. - // only collect exception string if this is the first attempt to fail fast on this thread. + // Only collect exception string if this is the first attempt to fail fast on this thread. alreadyFailing++; if (alreadyFailing != 1) {