Skip to content

Commit

Permalink
Revert "[Profiler] Make timer_create-based CPU profiler default" (#…
Browse files Browse the repository at this point in the history
…6579)

Reverts #6315

We have to revert because when threads are shutting down, we might face
crashes, deadlock because the CLR calls `pthread_exit`.

https://dev.azure.com/datadoghq/dd-trace-dotnet/_build/results?buildId=172042&view=logs&j=5635f724-ec42-5e82-7816-fb263b6cfcc0&t=e80b9b00-921f-539a-741c-f91926c040ef

TL;DR
`pthread_exit` is called by the CLR, which starts cleaning up the thread
thread_locals. But at the same time, the CPU `timer_create`-base kicks
in and tries using, indirectly, those thread_locals.
  • Loading branch information
gleocadie authored Jan 21, 2025
1 parent 7e973a6 commit c51efd8
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ std::string const Configuration::DefaultEmptyString = "";
std::chrono::seconds const Configuration::DefaultDevUploadInterval = 20s;
std::chrono::seconds const Configuration::DefaultProdUploadInterval = 60s;
std::chrono::milliseconds const Configuration::DefaultCpuProfilingInterval = 9ms;
CpuProfilerType const Configuration::DefaultCpuProfilerType =
#ifdef _WINDOWS
CpuProfilerType::ManualCpuTime;
#else
CpuProfilerType::TimerCreate;
#endif


Configuration::Configuration()
{
Expand Down Expand Up @@ -105,7 +98,7 @@ Configuration::Configuration()
_ssiLongLivedThreshold = ExtractSsiLongLivedThreshold();
_isTelemetryToDiskEnabled = GetEnvironmentValue(EnvironmentVariables::TelemetryToDiskEnabled, false);
_isSsiTelemetryEnabled = GetEnvironmentValue(EnvironmentVariables::SsiTelemetryEnabled, false);
_cpuProfilerType = GetEnvironmentValue(EnvironmentVariables::CpuProfilerType, DefaultCpuProfilerType);
_cpuProfilerType = GetEnvironmentValue(EnvironmentVariables::CpuProfilerType, CpuProfilerType::ManualCpuTime);
}

fs::path Configuration::ExtractLogDirectory()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ class Configuration final : public IConfiguration
static std::chrono::seconds const DefaultDevUploadInterval;
static std::chrono::seconds const DefaultProdUploadInterval;
static std::chrono::milliseconds const DefaultCpuProfilingInterval;
static CpuProfilerType const DefaultCpuProfilerType;

bool _isProfilingEnabled;
bool _isCpuProfilingEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,10 @@ public void CheckSignalHandlerIsInstalledOnce(string appName, string framework,
var logFile = Directory.GetFiles(runner.Environment.LogDir)
.Single(f => Path.GetFileName(f).StartsWith("DD-DotNet-Profiler-Native-"));

File.ReadLines(logFile)
.Count(l => l.Contains("Successfully setup signal handler for User defined signal 1 signal"))
.Should().Be(1);
var nbSignalHandlerInstallation = File.ReadLines(logFile)
.Count(l => l.Contains("Successfully setup signal handler for"));

File.ReadLines(logFile)
.Count(l => l.Contains("Successfully setup signal handler for Profiling timer expired signal"))
.Should().Be(1);
nbSignalHandlerInstallation.Should().Be(1);
}
}
}
24 changes: 3 additions & 21 deletions profiler/test/Datadog.Profiler.Native.Tests/ConfigurationTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1043,38 +1043,20 @@ TEST_F(ConfigurationTest, CheckDefaultCpuProfilerType)
{
EnvironmentHelper::EnvironmentVariable ar(EnvironmentVariables::CpuProfilerType, WStr(""));
auto configuration = Configuration{};
auto expected =
#ifdef _WINDOWS
CpuProfilerType::ManualCpuTime;
#else
CpuProfilerType::TimerCreate;
#endif
ASSERT_THAT(configuration.GetCpuProfilerType(), expected);
ASSERT_THAT(configuration.GetCpuProfilerType(), CpuProfilerType::ManualCpuTime);
}

TEST_F(ConfigurationTest, CheckDefaultCpuProfilerTypeWhenEnvVarNotSet)
{
auto configuration = Configuration{};
auto expected =
#ifdef _WINDOWS
CpuProfilerType::ManualCpuTime;
#else
CpuProfilerType::TimerCreate;
#endif
ASSERT_THAT(configuration.GetCpuProfilerType(), expected);
ASSERT_THAT(configuration.GetCpuProfilerType(), CpuProfilerType::ManualCpuTime);
}

TEST_F(ConfigurationTest, CheckUnknownCpuProfilerType)
{
EnvironmentHelper::EnvironmentVariable ar(EnvironmentVariables::CpuProfilerType, WStr("UnknownCpuProfilerType"));
auto configuration = Configuration{};
auto expected =
#ifdef _WINDOWS
CpuProfilerType::ManualCpuTime;
#else
CpuProfilerType::TimerCreate;
#endif
ASSERT_THAT(configuration.GetCpuProfilerType(), expected);
ASSERT_THAT(configuration.GetCpuProfilerType(), CpuProfilerType::ManualCpuTime);
}

TEST_F(ConfigurationTest, CheckManualCpuProfilerType)
Expand Down

0 comments on commit c51efd8

Please sign in to comment.