diff --git a/src/coreclr/vm/profilingenumerators.cpp b/src/coreclr/vm/profilingenumerators.cpp index f214be043f0bf8..50883da5ea8822 100644 --- a/src/coreclr/vm/profilingenumerators.cpp +++ b/src/coreclr/vm/profilingenumerators.cpp @@ -552,9 +552,11 @@ HRESULT ProfilerThreadEnum::Init() } CONTRACTL_END; + // If EnumThreads is called from a profiler callback where the runtime is already suspended, + // don't recursively acquire/release the ThreadStore Lock. // If a profiler has requested that the runtime suspend to do stack snapshots, it // will be holding the ThreadStore lock already - ThreadStoreLockHolder tsLock(!g_profControlBlock.fProfilerRequestedRuntimeSuspend); + ThreadStoreLockHolder tsLock(!ThreadStore::HoldingThreadStore() && !g_profControlBlock.fProfilerRequestedRuntimeSuspend); Thread * pThread = NULL; diff --git a/src/coreclr/vm/proftoeeinterfaceimpl.cpp b/src/coreclr/vm/proftoeeinterfaceimpl.cpp index 1d3530c615d1e5..bdf2648af9d7fa 100644 --- a/src/coreclr/vm/proftoeeinterfaceimpl.cpp +++ b/src/coreclr/vm/proftoeeinterfaceimpl.cpp @@ -6835,8 +6835,8 @@ HRESULT ProfToEEInterfaceImpl::SuspendRuntime() return CORPROF_E_SUSPENSION_IN_PROGRESS; } - g_profControlBlock.fProfilerRequestedRuntimeSuspend = TRUE; ThreadSuspend::SuspendEE(ThreadSuspend::SUSPEND_REASON::SUSPEND_FOR_PROFILER); + g_profControlBlock.fProfilerRequestedRuntimeSuspend = TRUE; return S_OK; } @@ -6874,8 +6874,8 @@ HRESULT ProfToEEInterfaceImpl::ResumeRuntime() return CORPROF_E_UNSUPPORTED_CALL_SEQUENCE; } - ThreadSuspend::RestartEE(FALSE /* bFinishedGC */, TRUE /* SuspendSucceeded */); g_profControlBlock.fProfilerRequestedRuntimeSuspend = FALSE; + ThreadSuspend::RestartEE(FALSE /* bFinishedGC */, TRUE /* SuspendSucceeded */); return S_OK; } @@ -7667,8 +7667,8 @@ HRESULT ProfToEEInterfaceImpl::EnumerateGCHeapObjects(ObjectCallback callback, v // SuspendEE() may race with other threads by design and this thread may block // arbitrarily long inside SuspendEE() for other threads to complete their own // suspensions. - g_profControlBlock.fProfilerRequestedRuntimeSuspend = TRUE; ThreadSuspend::SuspendEE(ThreadSuspend::SUSPEND_REASON::SUSPEND_FOR_PROFILER); + g_profControlBlock.fProfilerRequestedRuntimeSuspend = TRUE; ownEESuspension = TRUE; } @@ -7700,8 +7700,8 @@ HRESULT ProfToEEInterfaceImpl::EnumerateGCHeapObjects(ObjectCallback callback, v if (ownEESuspension) { - ThreadSuspend::RestartEE(FALSE /* bFinishedGC */, TRUE /* SuspendSucceeded */); g_profControlBlock.fProfilerRequestedRuntimeSuspend = FALSE; + ThreadSuspend::RestartEE(FALSE /* bFinishedGC */, TRUE /* SuspendSucceeded */); } return hr; diff --git a/src/tests/profiler/native/CMakeLists.txt b/src/tests/profiler/native/CMakeLists.txt index 1279874df07c32..4333a8b2698280 100644 --- a/src/tests/profiler/native/CMakeLists.txt +++ b/src/tests/profiler/native/CMakeLists.txt @@ -5,11 +5,11 @@ project(Profiler) set(SOURCES assemblyprofiler/assemblyprofiler.cpp eltprofiler/slowpatheltprofiler.cpp + enumthreadsprofiler/enumthreadsprofiler.cpp eventpipeprofiler/eventpipereadingprofiler.cpp eventpipeprofiler/eventpipewritingprofiler.cpp eventpipeprofiler/eventpipemetadatareader.cpp gcallocateprofiler/gcallocateprofiler.cpp - nongcheap/nongcheap.cpp gcbasicprofiler/gcbasicprofiler.cpp gcheapenumerationprofiler/gcheapenumerationprofiler.cpp gcheapenumerationprofiler/gcheapenumerationprofiler.def @@ -20,6 +20,7 @@ set(SOURCES metadatagetdispenser/metadatagetdispenser.cpp moduleload/moduleload.cpp multiple/multiple.cpp + nongcheap/nongcheap.cpp nullprofiler/nullprofiler.cpp rejitprofiler/rejitprofiler.cpp rejitprofiler/ilrewriter.cpp diff --git a/src/tests/profiler/native/classfactory.cpp b/src/tests/profiler/native/classfactory.cpp index bb27152f8581f8..677b57fef95efc 100644 --- a/src/tests/profiler/native/classfactory.cpp +++ b/src/tests/profiler/native/classfactory.cpp @@ -3,6 +3,7 @@ #include "classfactory.h" #include "eltprofiler/slowpatheltprofiler.h" +#include "enumthreadsprofiler/enumthreadsprofiler.h" #include "eventpipeprofiler/eventpipereadingprofiler.h" #include "eventpipeprofiler/eventpipewritingprofiler.h" #include "getappdomainstaticaddress/getappdomainstaticaddress.h" @@ -144,6 +145,10 @@ HRESULT STDMETHODCALLTYPE ClassFactory::CreateInstance(IUnknown *pUnkOuter, REFI { profiler = new GCHeapEnumerationProfiler(); } + else if (clsid == EnumThreadsProfiler::GetClsid()) + { + profiler = new EnumThreadsProfiler(); + } else { printf("No profiler found in ClassFactory::CreateInstance. Did you add your profiler to the list?\n"); diff --git a/src/tests/profiler/native/enumthreadsprofiler/enumthreadsprofiler.cpp b/src/tests/profiler/native/enumthreadsprofiler/enumthreadsprofiler.cpp new file mode 100644 index 00000000000000..de6caeaef7100a --- /dev/null +++ b/src/tests/profiler/native/enumthreadsprofiler/enumthreadsprofiler.cpp @@ -0,0 +1,104 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "enumthreadsprofiler.h" + +GUID EnumThreadsProfiler::GetClsid() +{ + // {0742962D-2ED3-44B0-BA84-06B1EF0A0A0B} + GUID clsid = { 0x0742962d, 0x2ed3, 0x44b0,{ 0xba, 0x84, 0x06, 0xb1, 0xef, 0x0a, 0x0a, 0x0b } }; + return clsid; +} + +HRESULT EnumThreadsProfiler::Initialize(IUnknown* pICorProfilerInfoUnk) +{ + Profiler::Initialize(pICorProfilerInfoUnk); + printf("EnumThreadsProfiler::Initialize\n"); + + HRESULT hr = S_OK; + if (FAILED(hr = pCorProfilerInfo->SetEventMask2(COR_PRF_MONITOR_GC | COR_PRF_MONITOR_SUSPENDS, COR_PRF_HIGH_MONITOR_NONE))) + { + printf("FAIL: ICorProfilerInfo::SetEventMask2() failed hr=0x%x", hr); + IncrementFailures(); + } + + return hr; +} + +HRESULT STDMETHODCALLTYPE EnumThreadsProfiler::GarbageCollectionStarted(int cGenerations, BOOL generationCollected[], COR_PRF_GC_REASON reason) +{ + SHUTDOWNGUARD(); + + printf("EnumThreadsProfiler::GarbageCollectionStarted\n"); + _gcStarts.fetch_add(1, std::memory_order_relaxed); + if (_gcStarts < _gcFinishes) + { + IncrementFailures(); + printf("EnumThreadsProfiler::GarbageCollectionStarted: FAIL: Expected GCStart >= GCFinish. Start=%d, Finish=%d\n", (int)_gcStarts, (int)_gcFinishes); + } + + return S_OK; +} + +HRESULT STDMETHODCALLTYPE EnumThreadsProfiler::GarbageCollectionFinished() +{ + SHUTDOWNGUARD(); + + printf("EnumThreadsProfiler::GarbageCollectionFinished\n"); + _gcFinishes.fetch_add(1, std::memory_order_relaxed); + if (_gcStarts < _gcFinishes) + { + IncrementFailures(); + printf("EnumThreadsProfiler::GarbageCollectionFinished: FAIL: Expected GCStart >= GCFinish. Start=%d, Finish=%d\n", (int)_gcStarts, (int)_gcFinishes); + } + + return S_OK; +} + +HRESULT STDMETHODCALLTYPE EnumThreadsProfiler::RuntimeSuspendFinished() +{ + SHUTDOWNGUARD(); + + printf("EnumThreadsProfiler::RuntimeSuspendFinished\n"); + + ICorProfilerThreadEnum* threadEnum = nullptr; + HRESULT enumThreadsHR = pCorProfilerInfo->EnumThreads(&threadEnum); + printf("Finished enumerating threads\n"); + _profilerEnumThreadsCompleted.fetch_add(1, std::memory_order_relaxed); + threadEnum->Release(); + return enumThreadsHR; +} + +HRESULT EnumThreadsProfiler::Shutdown() +{ + Profiler::Shutdown(); + + if (_gcStarts == 0) + { + printf("EnumThreadsProfiler::Shutdown: FAIL: Expected GarbageCollectionStarted to be called\n"); + } + else if (_gcFinishes == 0) + { + printf("EnumThreadsProfiler::Shutdown: FAIL: Expected GarbageCollectionFinished to be called\n"); + } + else if (_profilerEnumThreadsCompleted == 0) + { + printf("EnumThreadsProfiler::Shutdown: FAIL: Expected RuntimeSuspendFinished to be called and EnumThreads completed\n"); + } + else if(_failures == 0) + { + printf("PROFILER TEST PASSES\n"); + } + else + { + // failures were printed earlier when _failures was incremented + } + fflush(stdout); + + return S_OK; +} + +void EnumThreadsProfiler::IncrementFailures() +{ + _failures.fetch_add(1, std::memory_order_relaxed); +} diff --git a/src/tests/profiler/native/enumthreadsprofiler/enumthreadsprofiler.h b/src/tests/profiler/native/enumthreadsprofiler/enumthreadsprofiler.h new file mode 100644 index 00000000000000..df716108432594 --- /dev/null +++ b/src/tests/profiler/native/enumthreadsprofiler/enumthreadsprofiler.h @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#pragma once + +#include "../profiler.h" + +class EnumThreadsProfiler : public Profiler +{ +public: + EnumThreadsProfiler() : Profiler(), + _gcStarts(0), + _gcFinishes(0), + _profilerEnumThreadsCompleted(0), + _failures(0) + {} + + // Profiler callbacks override + static GUID GetClsid(); + virtual HRESULT STDMETHODCALLTYPE Initialize(IUnknown* pICorProfilerInfoUnk); + virtual HRESULT STDMETHODCALLTYPE GarbageCollectionStarted(int cGenerations, BOOL generationCollected[], COR_PRF_GC_REASON reason); + virtual HRESULT STDMETHODCALLTYPE GarbageCollectionFinished(); + virtual HRESULT STDMETHODCALLTYPE RuntimeSuspendFinished(); + virtual HRESULT STDMETHODCALLTYPE Shutdown(); + + // Helper methods + void IncrementFailures(); + +private: + std::atomic _gcStarts; + std::atomic _gcFinishes; + std::atomic _profilerEnumThreadsCompleted; + std::atomic _failures; +}; diff --git a/src/tests/profiler/unittest/enumthreads.cs b/src/tests/profiler/unittest/enumthreads.cs new file mode 100644 index 00000000000000..95562decc3f551 --- /dev/null +++ b/src/tests/profiler/unittest/enumthreads.cs @@ -0,0 +1,56 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Profiler.Tests +{ + class EnumThreadsTests + { + static readonly Guid EnumThreadsProfilerGuid = new Guid("0742962D-2ED3-44B0-BA84-06B1EF0A0A0B"); + + public static int EnumerateThreadsWithNonProfilerRequestedRuntimeSuspension() + { + GC.Collect(); + return 100; + } + + public static int Main(string[] args) + { + if (args.Length > 0 && args[0].Equals("RunTest", StringComparison.OrdinalIgnoreCase)) + { + switch (args[1]) + { + case nameof(EnumerateThreadsWithNonProfilerRequestedRuntimeSuspension): + return EnumerateThreadsWithNonProfilerRequestedRuntimeSuspension(); + default: + return 102; + } + } + + if (!RunProfilerTest(nameof(EnumerateThreadsWithNonProfilerRequestedRuntimeSuspension))) + { + return 101; + } + + return 100; + } + + private static bool RunProfilerTest(string testName) + { + try + { + return ProfilerTestRunner.Run(profileePath: System.Reflection.Assembly.GetExecutingAssembly().Location, + testName: "EnumThreads", + profilerClsid: EnumThreadsProfilerGuid, + profileeArguments: testName + ) == 100; + } + catch (Exception ex) + { + Console.WriteLine(ex); + } + return false; + } + } +} diff --git a/src/tests/profiler/unittest/enumthreads.csproj b/src/tests/profiler/unittest/enumthreads.csproj new file mode 100644 index 00000000000000..d51dcb692abfe0 --- /dev/null +++ b/src/tests/profiler/unittest/enumthreads.csproj @@ -0,0 +1,21 @@ + + + .NETCoreApp + exe + true + true + + true + + true + + + + + + + +