From 588ddc28d32fbcc6e6b83d4bfcde8079a2c37ba0 Mon Sep 17 00:00:00 2001 From: Gregory LEOCADIE Date: Mon, 17 Jun 2024 15:03:38 +0200 Subject: [PATCH] Fix delayed startup timer registration/deregistration --- .../ProfilerSignalManager.cpp | 4 ++-- .../TimerCreateCpuProfiler.cpp | 12 ++++++------ .../TimerCreateCpuProfiler.h | 2 +- .../Datadog.Profiler.Native/Configuration.cpp | 1 - .../Datadog.Profiler.Native/IManagedThreadList.h | 2 ++ .../Datadog.Profiler.Native/ManagedThreadList.cpp | 10 ++++++++++ .../Datadog.Profiler.Native/ManagedThreadList.h | 1 + .../Datadog.Profiler.Native/StackSamplerLoop.cpp | 4 +--- 8 files changed, 23 insertions(+), 13 deletions(-) diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/ProfilerSignalManager.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/ProfilerSignalManager.cpp index 2ef0263b2808..4150c1db067d 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/ProfilerSignalManager.cpp +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/ProfilerSignalManager.cpp @@ -165,12 +165,12 @@ bool ProfilerSignalManager::SetupSignalHandler() int32_t result = sigaction(_signalToSend, &sampleAction, &_previousAction); if (result != 0) { - Log::Error("ProfilerSignalManager::SetupSignalHandler: Failed to setup signal handler for ", _signalToSend, " signals. Reason: ", + Log::Error("ProfilerSignalManager::SetupSignalHandler: Failed to setup signal handler for ", strsignal(_signalToSend), " signals. Reason: ", strerror(errno), "."); return false; } - Log::Info("ProfilerSignalManager::SetupSignalHandler: Successfully setup signal handler for ", _signalToSend, " signal."); + Log::Info("ProfilerSignalManager::SetupSignalHandler: Successfully setup signal handler for ", strsignal(_signalToSend), " signal."); return true; } diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.cpp index 485aa1c7977b..e9213bdec538 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.cpp +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.cpp @@ -41,9 +41,9 @@ TimerCreateCpuProfiler::~TimerCreateCpuProfiler() void TimerCreateCpuProfiler::RegisterThread(std::shared_ptr threadInfo) { - std::shared_lock(_registerLock); + std::shared_lock lock(_registerLock); - if (GetState() != SeriveBase::State::Started) + if (GetState() != ServiceBase::State::Started) { return; } @@ -71,7 +71,7 @@ const char* TimerCreateCpuProfiler::GetName() bool TimerCreateCpuProfiler::StartImpl() { - // If the signal is higjacked, what to do? + // If the signal is highjacked, what to do? auto registered = _pSignalManager->RegisterHandler(TimerCreateCpuProfiler::CollectStackSampleSignalHandler); if (registered) @@ -79,8 +79,8 @@ bool TimerCreateCpuProfiler::StartImpl() std::unique_lock lock(_registerLock); Instance = this; - // Create and start timer for all threads - _pManagedThreadsList->ForEach([this](ManagedThreadList* thread) { RegisterImpl(thread); }); + // Create and start timer for all threads. + _pManagedThreadsList->ForEach([this](ManagedThreadInfo* thread) { RegisterThreadImpl(thread); }); } return registered; @@ -99,7 +99,7 @@ bool TimerCreateCpuProfiler::CollectStackSampleSignalHandler(int sig, siginfo_t* auto instance = Instance; if (instance == nullptr) { - return; + return false; } return instance->Collect(ucontext); diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.h b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.h index 0096662293a8..b7fe196a75f3 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.h +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native.Linux/TimerCreateCpuProfiler.h @@ -47,7 +47,7 @@ class TimerCreateCpuProfiler : public ServiceBase void RegisterThreadImpl(ManagedThreadInfo* thread); bool StartImpl() override; - bool StopImp() override; + bool StopImpl() override; enum class ServiceState { diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Configuration.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Configuration.cpp index 4b9f1bb3e644..3324361df427 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Configuration.cpp +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/Configuration.cpp @@ -581,7 +581,6 @@ CpuProfilerType Configuration::GetCpuProfilerType() const return _cpuProfilerType; } -======= std::chrono::milliseconds Configuration::GetCpuProfilingInterval() const { return _cpuProfilingInterval; diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/IManagedThreadList.h b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/IManagedThreadList.h index cf8c2f1e0276..8c5b0880095d 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/IManagedThreadList.h +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/IManagedThreadList.h @@ -8,6 +8,7 @@ #include "ServiceBase.h" #include "ManagedThreadInfo.h" +#include #include class IManagedThreadList : public ServiceBase @@ -25,4 +26,5 @@ class IManagedThreadList : public ServiceBase virtual HRESULT TryGetCurrentThreadInfo(std::shared_ptr& ppThreadInfo) = 0; virtual std::shared_ptr GetOrCreate(ThreadID clrThreadId) = 0; virtual bool TryGetThreadInfo(uint32_t osThreadId, std::shared_ptr& ppThreadInfo) = 0; + virtual void ForEach(std::function callback) = 0; }; \ No newline at end of file diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.cpp index a2c8525e919d..9910c687ccb7 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.cpp +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.cpp @@ -364,4 +364,14 @@ bool ManagedThreadList::RegisterThread(std::shared_ptr& pThre } return false; +} + +void ManagedThreadList::ForEach(std::function callback) +{ + std::lock_guard lock(_mutex); + + for (auto& thread : _threads) + { + callback(thread.get()); + } } \ No newline at end of file diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.h b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.h index ce62fac4ff13..137fb493902a 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.h +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/ManagedThreadList.h @@ -40,6 +40,7 @@ class ManagedThreadList : public IManagedThreadList HRESULT TryGetCurrentThreadInfo(std::shared_ptr& ppThreadInfo) override; std::shared_ptr GetOrCreate(ThreadID clrThreadId) override; bool TryGetThreadInfo(uint32_t osThreadId, std::shared_ptr& ppThreadInfo) override; + void ForEach(std::function callback) override; private: const char* _serviceName = "ManagedThreadList"; diff --git a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/StackSamplerLoop.cpp b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/StackSamplerLoop.cpp index 4fc6b7fdc437..da244855ac05 100644 --- a/profiler/src/ProfilerEngine/Datadog.Profiler.Native/StackSamplerLoop.cpp +++ b/profiler/src/ProfilerEngine/Datadog.Profiler.Native/StackSamplerLoop.cpp @@ -64,10 +64,8 @@ StackSamplerLoop::StackSamplerLoop( _cpuThreadsThreshold{pConfiguration->CpuThreadsThreshold()}, _codeHotspotsThreadsThreshold{pConfiguration->CodeHotspotsThreadsThreshold()}, _isWalltimeEnabled{pConfiguration->IsWallTimeProfilingEnabled()}, - _isCpuEnabled{pConfiguration->IsCpuProfilingEnabled()}, - _areInternalMetricsEnabled{pConfiguration->IsInternalMetricsEnabled()} _isCpuEnabled{pConfiguration->IsCpuProfilingEnabled() && pConfiguration->GetCpuProfilerType() == CpuProfilerType::ManualCpuTime}, - _areInternalMetricsEnabled{pConfiguration->IsInternalMetricsEnabled()}, + _areInternalMetricsEnabled{pConfiguration->IsInternalMetricsEnabled()} { _nbCores = OsSpecificApi::GetProcessorCount(); Log::Info("Processor cores = ", _nbCores);