Skip to content

Avoid exceptions in ProcessBasicChecksTests #7781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 11, 2025
Merged

Avoid exceptions in ProcessBasicChecksTests #7781

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9dba210
Add a lock to avoid exceptions
NachoEchevarria Nov 5, 2025
6db0aa8
Move lock to ConsoleTestHelper
NachoEchevarria Nov 5, 2025
ed6ac5a
Apply corflags only once
NachoEchevarria Nov 6, 2025
e59735c
Make dictionary case insensitive
NachoEchevarria Nov 6, 2025
bb295b8
Remove redundant comments
NachoEchevarria Nov 6, 2025
2db74e5
Single lock
NachoEchevarria Nov 6, 2025
d166e4f
Dispose process
NachoEchevarria Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 56 additions & 34 deletions tracer/test/Datadog.Trace.TestHelpers/ProfilerHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// </copyright>

using System;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.IO;
using System.Linq;
Expand All @@ -19,6 +20,8 @@ public class ProfilerHelper
.Concat(new[] { "dotnet", "iisexpress" })
.ToArray();

private static readonly object _lockObj = new();
private static readonly ConcurrentDictionary<string, bool> _corFlagsApplied = new ConcurrentDictionary<string, bool>(StringComparer.OrdinalIgnoreCase);
Copy link
Member

@andrewlock andrewlock Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should we ignore case 🤔 On Linux for example there could be two different samples with the same name, just different cases 😅 I don't think it's a problem in reality though

Copy link
Collaborator Author

@NachoEchevarria NachoEchevarria Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we only change cor vars in Windows x86 net4, so I would keep it

private static string _corFlagsExe;

public static async Task<Process> StartProcessWithProfiler(
Expand Down Expand Up @@ -89,53 +92,72 @@ public static async Task<Process> StartProcessWithProfiler(

public static void SetCorFlags(string executable, ITestOutputHelper output, bool require32Bit)
{
var corFlagsExe = _corFlagsExe;
var setBit = require32Bit ? "/32BITREQ+" : "/32BITREQ-";
if (string.IsNullOrEmpty(corFlagsExe))
// Use the absolute path as the key to track which executables have been modified
var executablePath = Path.GetFullPath(executable);

if (_corFlagsApplied.TryGetValue(executablePath, out _))
{
var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
var dotnetWindowsSdkToolsFolder = Path.Combine(programFiles, "Microsoft SDKs", "Windows", "v10.0A", "bin");
return;
}

lock (_lockObj)
{
// check again to be safe
if (_corFlagsApplied.TryGetValue(executablePath, out _))
{
return;
}

output?.WriteLine($"Searching for CorFlags.exe in {dotnetWindowsSdkToolsFolder}");
if (Directory.Exists(dotnetWindowsSdkToolsFolder))
var corFlagsExe = _corFlagsExe;
var setBit = require32Bit ? "/32BITREQ+" : "/32BITREQ-";
if (string.IsNullOrEmpty(corFlagsExe))
{
// get sub directories, e.g.
// @"C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8.1 Tools",
// @"C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools",
foreach (var folder in Directory.EnumerateDirectories(dotnetWindowsSdkToolsFolder))
var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
var dotnetWindowsSdkToolsFolder = Path.Combine(programFiles, "Microsoft SDKs", "Windows", "v10.0A", "bin");

output?.WriteLine($"Searching for CorFlags.exe in {dotnetWindowsSdkToolsFolder}");
if (Directory.Exists(dotnetWindowsSdkToolsFolder))
{
var exe = Path.Combine(folder, "x64", "CorFlags.exe");
if (File.Exists(exe))
// get sub directories, e.g.
// @"C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8.1 Tools",
// @"C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools",
foreach (var folder in Directory.EnumerateDirectories(dotnetWindowsSdkToolsFolder))
{
corFlagsExe = exe;
break;
var exe = Path.Combine(folder, "x64", "CorFlags.exe");
if (File.Exists(exe))
{
corFlagsExe = exe;
break;
}
}
}
}

if (!string.IsNullOrEmpty(corFlagsExe))
{
Interlocked.Exchange(ref _corFlagsExe, corFlagsExe);
output?.WriteLine($"CorFlags.exe found at {corFlagsExe}");
if (!string.IsNullOrEmpty(corFlagsExe))
{
Interlocked.Exchange(ref _corFlagsExe, corFlagsExe);
output?.WriteLine($"CorFlags.exe found at {corFlagsExe}");
}
else
{
throw new Exception($"Could not find CorFlags.exe so unable to set {setBit}");
}
}
else

output?.WriteLine($"Updating {Path.GetFileName(executable)} using {setBit}");
var opts = new ProcessStartInfo(corFlagsExe, $"\"{executable}\" {setBit} /force")
{
throw new Exception($"Could not find CorFlags.exe so unable to set {setBit}");
}
}
CreateNoWindow = true,
UseShellExecute = false
};

output?.WriteLine($"Updating {Path.GetFileName(executable)} using {setBit}");
var opts = new ProcessStartInfo(corFlagsExe, $"\"{executable}\" {setBit} /force")
{
CreateNoWindow = true,
UseShellExecute = false
};
var executedSuccessfully = Process.Start(opts).WaitForExit(20_000);
Copy link
Collaborator

@bouwkast bouwkast Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think we should dispose of the Process from Process.Start?

It was already like this though so 🤷

Copy link
Collaborator Author

@NachoEchevarria NachoEchevarria Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I have modified the code. Even thought it was already like that, it's better to dispose it. Thanks!


var executedSuccessfully = Process.Start(opts).WaitForExit(20_000);
if (!executedSuccessfully)
{
throw new Exception($"Error setting CorFlags.exe {Path.GetFileName(executable)} {setBit}");
}

if (!executedSuccessfully)
{
throw new Exception($"Error setting CorFlags.exe {Path.GetFileName(executable)} {setBit}");
_corFlagsApplied.TryAdd(executablePath, true);
}
}
}
Expand Down
Loading