Fix a contract violation in ThreadPool.get_CompletedWorkItemCount (dotnet/coreclr...

Fix a contract violation in ThreadPool.get_CompletedWorkItemCount

Fixes https://github.com/dotnet/coreclr/issues/24515
- Looks like the path that acquires the thread store lock has code that is marked witH GC_TRIGGERS
- Changed ThreadPool.get_CompletedWorkItemCount and Monitor.get_LockContentionCount to use QCalls instead of FCalls

Commit migrated from https://github.com/dotnet/coreclr/commit/fd2287c5e833439e0ea7c27ad14350bcccf371a0

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Monitor.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Monitor.cs
index a123f06..4385bf9 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Monitor.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Monitor.cs
@@ -20,6 +20,7 @@ using System.Runtime.CompilerServices;
 using System.Runtime.ConstrainedExecution;
 using System.Runtime.Versioning;
 using System.Diagnostics;
+using System.Runtime.InteropServices;
 
 namespace System.Threading
 {
@@ -236,10 +237,9 @@ namespace System.Threading
         /// <summary>
         /// Gets the number of times there was contention upon trying to take a <see cref="Monitor"/>'s lock so far.
         /// </summary>
-        public static extern long LockContentionCount
-        {
-            [MethodImpl(MethodImplOptions.InternalCall)]
-            get;
-        }
+        public static long LockContentionCount => GetLockContentionCount();
+
+        [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
+        private static extern long GetLockContentionCount();
     }
 }

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs
index d7b2c27..eac84d4 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs
@@ -254,11 +254,10 @@ namespace System.Threading
         /// <remarks>
         /// For a thread pool implementation that may have different types of work items, the count includes all types.
         /// </remarks>
-        public static extern long CompletedWorkItemCount
-        {
-            [MethodImpl(MethodImplOptions.InternalCall)]
-            get;
-        }
+        public static long CompletedWorkItemCount => GetCompletedWorkItemCount();
+
+        [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
+        private static extern long GetCompletedWorkItemCount();
 
         private static extern long PendingUnmanagedWorkItemCount
         {

diff --git a/src/coreclr/src/classlibnative/bcltype/objectnative.cpp b/src/coreclr/src/classlibnative/bcltype/objectnative.cpp
index 14e4cee..dc4fd92 100644 (file)
--- a/src/coreclr/src/classlibnative/bcltype/objectnative.cpp
+++ b/src/coreclr/src/classlibnative/bcltype/objectnative.cpp
@@ -342,9 +342,16 @@ FCIMPL1(FC_BOOL_RET, ObjectNative::IsLockHeld, Object* pThisUNSAFE)
 }
 FCIMPLEND
 
-FCIMPL0(INT64, ObjectNative::GetMonitorLockContentionCount)
+INT64 QCALLTYPE ObjectNative::GetMonitorLockContentionCount()
 {
-    FCALL_CONTRACT;
-    return (INT64)Thread::GetTotalMonitorLockContentionCount();
+    QCALL_CONTRACT;
+
+    INT64 result = 0;
+
+    BEGIN_QCALL;
+
+    result = (INT64)Thread::GetTotalMonitorLockContentionCount();
+
+    END_QCALL;
+    return result;
 }
-FCIMPLEND

diff --git a/src/coreclr/src/classlibnative/bcltype/objectnative.h b/src/coreclr/src/classlibnative/bcltype/objectnative.h
index 3d008d9..f5583cf 100644 (file)
--- a/src/coreclr/src/classlibnative/bcltype/objectnative.h
+++ b/src/coreclr/src/classlibnative/bcltype/objectnative.h
@@ -38,7 +38,7 @@ public:
     static FCDECL1(void, Pulse, Object* pThisUNSAFE);
     static FCDECL1(void, PulseAll, Object* pThisUNSAFE);
     static FCDECL1(FC_BOOL_RET, IsLockHeld, Object* pThisUNSAFE);
-    static FCDECL0(INT64, GetMonitorLockContentionCount);
+    static INT64 QCALLTYPE GetMonitorLockContentionCount();
 };
 
 #endif // _OBJECTNATIVE_H_

diff --git a/src/coreclr/src/vm/comthreadpool.cpp b/src/coreclr/src/vm/comthreadpool.cpp
index f5dc423..8a1f887 100644 (file)
--- a/src/coreclr/src/vm/comthreadpool.cpp
+++ b/src/coreclr/src/vm/comthreadpool.cpp
@@ -190,12 +190,19 @@ FCIMPL0(INT32, ThreadPoolNative::GetThreadCount)
 FCIMPLEND
 
 /*****************************************************************************************************/
-FCIMPL0(INT64, ThreadPoolNative::GetCompletedWorkItemCount)
+INT64 QCALLTYPE ThreadPoolNative::GetCompletedWorkItemCount()
 {
-    FCALL_CONTRACT;
-    return (INT64)Thread::GetTotalThreadPoolCompletionCount();
+    QCALL_CONTRACT;
+
+    INT64 result = 0;
+
+    BEGIN_QCALL;
+
+    result = (INT64)Thread::GetTotalThreadPoolCompletionCount();
+
+    END_QCALL;
+    return result;
 }
-FCIMPLEND
 
 /*****************************************************************************************************/
 FCIMPL0(INT64, ThreadPoolNative::GetPendingUnmanagedWorkItemCount)

diff --git a/src/coreclr/src/vm/comthreadpool.h b/src/coreclr/src/vm/comthreadpool.h
index d830c9e..aaa1b3b 100644 (file)
--- a/src/coreclr/src/vm/comthreadpool.h
+++ b/src/coreclr/src/vm/comthreadpool.h
@@ -29,7 +29,7 @@ public:
     static FCDECL2(VOID, CorGetMinThreads, DWORD* workerThreads, DWORD* completionPortThreads);
     static FCDECL2(VOID, CorGetAvailableThreads, DWORD* workerThreads, DWORD* completionPortThreads);
     static FCDECL0(INT32, GetThreadCount);
-    static FCDECL0(INT64, GetCompletedWorkItemCount);
+    static INT64 QCALLTYPE GetCompletedWorkItemCount();
     static FCDECL0(INT64, GetPendingUnmanagedWorkItemCount);
 
     static FCDECL0(VOID, NotifyRequestProgress);

diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h
index 89c05a5..fa616b8 100644 (file)
--- a/src/coreclr/src/vm/ecalllist.h
+++ b/src/coreclr/src/vm/ecalllist.h
@@ -673,7 +673,7 @@ FCFuncStart(gThreadPoolFuncs)
     FCFuncElement("SetMinThreadsNative", ThreadPoolNative::CorSetMinThreads)
     FCFuncElement("GetMinThreadsNative", ThreadPoolNative::CorGetMinThreads)
     FCFuncElement("get_ThreadCount", ThreadPoolNative::GetThreadCount)
-    FCFuncElement("get_CompletedWorkItemCount", ThreadPoolNative::GetCompletedWorkItemCount)
+    QCFuncElement("GetCompletedWorkItemCount", ThreadPoolNative::GetCompletedWorkItemCount)
     FCFuncElement("get_PendingUnmanagedWorkItemCount", ThreadPoolNative::GetPendingUnmanagedWorkItemCount)
     FCFuncElement("RegisterWaitForSingleObjectNative", ThreadPoolNative::CorRegisterWaitForSingleObject)
     FCFuncElement("BindIOCompletionCallbackNative", ThreadPoolNative::CorBindIoCompletionCallback)
@@ -915,7 +915,7 @@ FCFuncStart(gMonitorFuncs)
     FCFuncElement("ObjPulse", ObjectNative::Pulse)
     FCFuncElement("ObjPulseAll", ObjectNative::PulseAll)
     FCFuncElement("IsEnteredNative", ObjectNative::IsLockHeld)
-    FCFuncElement("get_LockContentionCount", ObjectNative::GetMonitorLockContentionCount)
+    QCFuncElement("GetLockContentionCount", ObjectNative::GetMonitorLockContentionCount)
 FCFuncEnd()
 
 FCFuncStart(gOverlappedFuncs)

diff --git a/src/coreclr/src/vm/threads.cpp b/src/coreclr/src/vm/threads.cpp
index 715b222..beb09f2 100644 (file)
--- a/src/coreclr/src/vm/threads.cpp
+++ b/src/coreclr/src/vm/threads.cpp
@@ -8000,13 +8000,14 @@ NOINLINE void Thread::OnIncrementCountOverflow(UINT32 *threadLocalCount, UINT64
 
 UINT64 Thread::GetTotalCount(SIZE_T threadLocalCountOffset, UINT64 *overflowCount)
 {
-    CONTRACTL
-    {
+    CONTRACTL {
         NOTHROW;
-        MODE_ANY;
+        GC_TRIGGERS;
     }
     CONTRACTL_END;
 
+    _ASSERTE(overflowCount != nullptr);
+
     // enumerate all threads, summing their local counts.
     ThreadStoreLockHolder tsl;
 
@@ -8023,10 +8024,9 @@ UINT64 Thread::GetTotalCount(SIZE_T threadLocalCountOffset, UINT64 *overflowCoun
 
 UINT64 Thread::GetTotalThreadPoolCompletionCount()
 {
-    CONTRACTL
-    {
+    CONTRACTL {
         NOTHROW;
-        MODE_ANY;
+        GC_TRIGGERS;
     }
     CONTRACTL_END;