Skip to content

[IAST] Fix racing conditions in Dataflow #7838

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

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[IAST] Fix racing conditions in Dataflow #7838

wants to merge 5 commits into from
+60 −17

Conversation

@NachoEchevarria
Copy link
Collaborator

@NachoEchevarria NachoEchevarria commented Nov 18, 2025 •
edited
Loading

Summary of changes

This PR fixes multi-threading issues in Dataflow that could lead to corruption of the internal std::map structures (_modules, _appDomains, _moduleAspects) and subsequent crashes inside MSVC STL (_Tree::_Find_lower_bound). These issues were observed as flaky CI failures on Windows x86, manifesting as:

2025-11-10T12:05:43.1623592Z 12:05:43 [DBG]  Standard Output Messages:
2025-11-10T12:05:43.1624040Z 12:05:43 [DBG]  Platform: X86
2025-11-10T12:05:43.1624418Z 12:05:43 [DBG]  TargetPlatform: X86
2025-11-10T12:05:43.1624695Z 12:05:43 [DBG]  Configuration: Release
2025-11-10T12:05:43.1624968Z 12:05:43 [DBG]  TargetFramework: net7.0
2025-11-10T12:05:43.1625239Z 12:05:43 [DBG]  .NET Core: True
2025-11-10T12:05:43.1625595Z 12:05:43 [DBG]  Native Loader DLL: D:\a\_work\1\s\shared\bin\monitoring-home\win-x86\Datadog.Trace.ClrProfiler.Native.dll
2025-11-10T12:05:43.1625940Z 12:05:43 [DBG]  Starting aspnetcore sample, agentPort: 53110
2025-11-10T12:05:43.1626334Z 12:05:43 [DBG]  Starting Application: D:\a\_work\1\s\artifacts\bin\Samples.Security.AspNetCore5\release_net7.0\Samples.Security.AspNetCore5.dll
2025-11-10T12:05:43.1626821Z 12:05:43 [DBG]  ProcessId: 4216
2025-11-10T12:05:43.1627140Z 12:05:43 [DBG]  [webserver][stderr] Fatal error. Internal CLR error. (0x80131506)
2025-11-10T12:05:43.1627502Z 12:05:43 [DBG]  [webserver][stderr]    at System.Threading.PortableThreadPool+GateThread.GateThreadStart()
2025-11-10T12:05:43.1627849Z 12:05:43 [DBG]  [webserver][stderr]    at System.Threading.Thread.StartCallback()
2025-11-10T12:05:43.1628229Z 12:05:43 [DBG]  [webserver][stdout] [createdump] Writing full dump for process 4216 to file C:\Users\AzDevOps\AppData\Local\Temp\dump.4216.dmp
2025-11-10T12:05:43.1628594Z 12:05:43 [DBG]  [webserver][stdout] [createdump] Dump successfully written in 11266ms
2025-11-10T12:05:43.1628956Z 12:05:43 [DBG]  [webserver][stdout] info: Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager[63]
2025-11-10T12:05:43.1629405Z 12:05:43 [DBG]  [webserver][stdout]       User profile is available. Using 'C:\Users\AzDevOps\AppData\Local\ASP.NET\DataProtection-Keys' as key repository and Windows DPAPI to encrypt keys at rest.
2025-11-10T12:05:43.1629853Z 12:05:43 [DBG]  [webserver][stdout] info: Microsoft.Hosting.Lifetime[14]
2025-11-10T12:05:43.1630499Z 12:05:43 [DBG]  [webserver][stdout]       Now listening on: http://127.0.0.1:53179
2025-11-10T12:05:43.1630959Z 12:05:43 [DBG]  [webserver][stdout] info: Microsoft.Hosting.Lifetime[0]
2025-11-10T12:05:43.1631295Z 12:05:43 [DBG]  [webserver][stdout]       Application started. Press Ctrl+C to shut down.

Inspecting the dump, we get the following stack:

02d7e2d8 6ac018b6     02d7d760 02d7d864 b928276d Datadog_Tracer_Native!std::_Tree<std::_Tset_traits<unsigned int,std::less<unsigned int>,std::allocator<unsigned int>,0> >::_Find_lower_bound<unsigned int>+0x33
(Inline) --------     -------- -------- -------- Datadog_Tracer_Native!std::_Tree<std::_Tmap_traits<unsigned int,iast::ModuleInfo *,std::less<unsigned int>,std::allocator<std::pair<unsigned int const ,iast::ModuleInfo *> >,0> >::_Find+0x1f
(Inline) --------     -------- -------- -------- Datadog_Tracer_Native!std::_Tree<std::_Tmap_traits<unsigned int,iast::ModuleInfo *,std::less<unsigned int>,std::allocator<std::pair<unsigned int const ,iast::ModuleInfo *> >,0> >::find+0x1f
02d7e2d8 6ac01f24     2be9035c 034fbf20 02d7e7d4 Datadog_Tracer_Native!iast::Dataflow::GetModuleInfo+0xc6
02d7e2f8 6abb96b4     2be9035c 0600001d 00000000 Datadog_Tracer_Native!iast::Dataflow::JITProcessMethod+0x24
(Inline) --------     -------- -------- -------- Datadog_Tracer_Native!iast::Dataflow::IsInlineEnabled+0x16
02d7e54c 6ade76d8     034fbf20 2be90824 02d7e7d4 Datadog_Tracer_Native!trace::CorProfiler::JITCachedFunctionSearchStarted+0x1e4
02d7e5b8 2b0644d8     000002ad 00000000 02a302f7 Datadog_Trace_ClrProfiler_Native+0x476d8
00000000 00000000     00000000 00000000 00000000 0x2b0644d8

This is where the exception was thrown, but not necessarily where the problem is. The map could get corrupted earlier and later the exception thrown in some internal checks.

Root Causes

The Dataflow instance is accessed from multiple CLR profiler callbacks (JITCachedFunctionSearchStarted, JITCompilationStarted, module load/unload, ReJIT), which execute on various .NET threads. While most code paths synchronize access to shared maps via _cs, several critical issues existed:

1. Unsynchronized Destructor

Dataflow::~Dataflow() modified _modules, _appDomains, and _moduleAspects without holding _cs, allowing races where:

  • One thread was destructing/clearing the maps
  • Another thread entered methods like GetModuleInfo() under the lock
  • Result: STL tree corruption and crashes

2. Time-of-Check-Time-of-Use (TOCTOU) Race Conditions

Multiple methods checked _initialized outside the critical section, then accessed shared maps inside the lock:

  • GetAppDomain(), GetModuleInfo(), GetAspectsModule()
  • AppDomainShutdown(), ModuleUnloaded()
  • GetAspects()

This created a race window where:

  1. Thread A checks IsInitialized() → returns TRUE (no lock held)
  2. Thread B enters destructor, acquires lock, sets _initialized = false, destroys all maps
  3. Thread A acquires lock, accesses destroyed maps → CRASH

3. Non-Atomic _initialized Flag

_initialized was a plain bool accessed concurrently without synchronization, causing:

  • Data races (undefined behavior in C++)
  • Lack of memory ordering guarantees between destructor and callbacks

4. Unsynchronized Map Access in LoadAspects()

LoadAspects() cleared _moduleAspects without locking.

Fixes Included in This PR

1. Eliminate TOCTOU Races - Move Checks Inside Critical Sections

All methods that directly access shared maps now check IsInitialized() inside the CSGUARD(_cs) lock:

  • ~Dataflow() - Acquires lock first, then sets flag and destroys maps atomically
  • GetAppDomain() - Lock → check → access
  • GetModuleInfo() - Lock → check → access
  • AppDomainShutdown() - Lock → check → access
  • ModuleUnloaded() - Lock → check → access
  • GetAspects() - Lock → check → access

This ensures that once the destructor sets _initialized = false inside the lock, no other thread can proceed past the check to access destroyed state.

Before (vulnerable):

if (!_isInitialized)    // Thread A checks outside lock → TRUE
    return;
                         // Thread B: lock → set false → destroy maps → unlock
CSGUARD(_cs);            // Thread A: acquires lock
auto x = _modules.find() // Thread A: accesses destroyed map → CRASH

After (safe):

CSGUARD(_cs);            // Thread A: acquires lock
if (!IsInitialized())    // Thread A: checks inside lock
    return;              // If Thread B already set false, exits safely
auto x = _modules.find() // Only proceeds if initialized

2. Atomic _initialized with Proper Memory Ordering

Changed _initialized from bool to std::atomic<bool> with:

  • memory_order_acquire for loads (in IsInitialized())
  • memory_order_release for stores (in SetInitialized())

This ensures:

  • No data races (well-defined behavior)
  • Proper memory visibility: when a thread sees _initialized == false, it's guaranteed to see all prior writes made before the release store
  • Required because many methods still check IsInitialized() outside locks (e.g., IsInlineEnabled(), JITProcessMethod()) but safely delegate to methods that lock properly

3. Inline Helper Methods in Header

Added IsInitialized() and SetInitialized() to the header file as inline methods for:

  • Better code generation (inlining)
  • Consistent atomic access patterns
  • Clear abstraction over atomic operations

4. Optimize LoadAspects() Locking

Updated LoadAspects() to:

  • Hold _cs lock only for the swap() operation on _moduleAspects
  • Destroy old ModuleAspects objects outside the lock
  • Minimizes lock contention and prevents potential deadlocks

5. Remove Redundant Lock in GetAspectsModule()

Removed CSGUARD(_cs) from GetAspectsModule() since it delegates to GetModuleInfo(), which now properly locks. Avoids unnecessary nested locking (though safe due to recursive CRITICAL_SECTION on Windows).

Reason for change

Fix critical race conditions causing flaky crashes in CI, particularly on Windows x86. The crashes manifested as STL internal errors during concurrent access to Dataflow's internal maps during JIT compilation and shutdown scenarios.

This error could also happen in customer installations, causing crashes.

Implementation details

Files changed:

  • tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp - TOCTOU fixes, lock reordering, destructor synchronization
  • tracer/src/Datadog.Tracer.Native/iast/dataflow.h - Atomic flag declaration, inline helper methods with acquire/release semantics

Key synchronization pattern:
The fix leverages the recursive nature of Windows CRITICAL_SECTION (used by CSGUARD) while eliminating TOCTOU races by ensuring all shared state access happens atomically with the initialization check under the same lock.

Methods updated:

  • ~Dataflow() - Lock before setting flag and destroying maps
  • GetAppDomain(), GetModuleInfo(), GetAspectsModule() - Check inside lock
  • AppDomainShutdown(), ModuleUnloaded(), GetAspects() - Check inside lock
  • Added IsInitialized()/SetInitialized() - Atomic with acquire/release memory ordering

Test coverage

This fix addresses the root cause of the flaky Windows x86 CI failures. The existing CI pipeline validates the fix through:

  • Multi-threaded scenarios with concurrent JIT compilation and module loading
  • Windows x86 integration tests (where the issue was originally observed)
  • AppSec security tests that exercise IAST Dataflow heavily

Other details

@dd-trace-dotnet-ci-bot
Copy link

dd-trace-dotnet-ci-bot bot commented Nov 18, 2025 •
edited
Loading

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing This PR (7838) and master.

✅ No regressions detected - check the details below

Full Metrics Comparison

FakeDbCommand

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration75.22 ± (74.81 - 75.42) ms76.22 ± (76.53 - 77.67) ms+1.3%✅⬆️
.NET Framework 4.8 - Bailout
duration79.04 ± (79.02 - 79.72) ms80.99 ± (80.80 - 81.79) ms+2.5%✅⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1121.02 ± (1127.17 - 1137.93) ms1138.31 ± (1143.59 - 1156.79) ms+1.5%✅⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms22.94 ± (22.86 - 23.02) ms22.89 ± (22.80 - 22.98) ms-0.2%✅
process.time_to_main_ms87.88 ± (87.55 - 88.22) ms87.38 ± (86.94 - 87.82) ms-0.6%✅
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed10.93 ± (10.92 - 10.93) MB10.92 ± (10.91 - 10.92) MB-0.1%✅
runtime.dotnet.threads.count12 ± (12 - 12)12 ± (12 - 12)+0.0%✅
.NET Core 3.1 - Bailout
process.internal_duration_ms23.03 ± (22.95 - 23.11) ms22.77 ± (22.70 - 22.83) ms-1.2%✅
process.time_to_main_ms90.06 ± (89.69 - 90.43) ms88.72 ± (88.25 - 89.19) ms-1.5%✅
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed10.97 ± (10.97 - 10.98) MB10.95 ± (10.95 - 10.95) MB-0.2%✅
runtime.dotnet.threads.count13 ± (13 - 13)13 ± (13 - 13)+0.0%✅
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms220.97 ± (219.51 - 222.43) ms221.69 ± (220.39 - 222.99) ms+0.3%✅⬆️
process.time_to_main_ms550.54 ± (549.37 - 551.71) ms552.98 ± (551.55 - 554.41) ms+0.4%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed52.54 ± (52.52 - 52.56) MB52.49 ± (52.47 - 52.51) MB-0.1%✅
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)+0.1%✅⬆️
.NET 6 - Baseline
process.internal_duration_ms22.11 ± (22.05 - 22.18) ms21.90 ± (21.81 - 21.98) ms-1.0%✅
process.time_to_main_ms78.46 ± (78.11 - 78.81) ms75.69 ± (75.30 - 76.08) ms-3.5%✅
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed10.62 ± (10.61 - 10.62) MB10.62 ± (10.61 - 10.62) MB+0.0%✅⬆️
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%✅
.NET 6 - Bailout
process.internal_duration_ms21.61 ± (21.55 - 21.67) ms21.67 ± (21.60 - 21.74) ms+0.3%✅⬆️
process.time_to_main_ms77.44 ± (77.18 - 77.71) ms77.36 ± (76.91 - 77.81) ms-0.1%✅
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed10.66 ± (10.65 - 10.66) MB10.72 ± (10.72 - 10.72) MB+0.6%✅⬆️
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%✅
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms207.34 ± (206.12 - 208.57) ms209.78 ± (208.39 - 211.18) ms+1.2%✅⬆️
process.time_to_main_ms517.03 ± (516.10 - 517.95) ms513.24 ± (511.78 - 514.70) ms-0.7%✅
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed51.63 ± (51.61 - 51.66) MB51.60 ± (51.57 - 51.62) MB-0.1%✅
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)+0.0%✅⬆️
.NET 8 - Baseline
process.internal_duration_ms19.98 ± (19.92 - 20.04) ms20.06 ± (19.99 - 20.13) ms+0.4%✅⬆️
process.time_to_main_ms75.31 ± (75.03 - 75.59) ms75.15 ± (74.82 - 75.48) ms-0.2%✅
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed7.69 ± (7.68 - 7.69) MB7.65 ± (7.64 - 7.65) MB-0.5%✅
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%✅
.NET 8 - Bailout
process.internal_duration_ms20.01 ± (19.94 - 20.08) ms20.11 ± (20.02 - 20.19) ms+0.5%✅⬆️
process.time_to_main_ms76.33 ± (76.04 - 76.63) ms76.87 ± (76.38 - 77.35) ms+0.7%✅⬆️
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed7.74 ± (7.73 - 7.74) MB7.70 ± (7.70 - 7.71) MB-0.4%✅
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%✅
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms193.28 ± (192.36 - 194.20) ms192.27 ± (191.26 - 193.27) ms-0.5%✅
process.time_to_main_ms494.33 ± (493.35 - 495.31) ms490.95 ± (489.63 - 492.26) ms-0.7%✅
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%✅
runtime.dotnet.mem.committed38.88 ± (38.84 - 38.92) MB38.93 ± (38.89 - 38.96) MB+0.1%✅⬆️
runtime.dotnet.threads.count27 ± (27 - 27)27 ± (27 - 27)+0.0%✅⬆️

HttpMessageHandler

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration191.59 ± (191.42 - 192.21) ms199.86 ± (199.90 - 200.90) ms+4.3%✅⬆️
.NET Framework 4.8 - Bailout
duration195.01 ± (194.89 - 195.35) ms200.80 ± (200.50 - 201.32) ms+3.0%✅⬆️
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1165.85 ± (1170.07 - 1179.54) ms1178.92 ± (1181.84 - 1192.04) ms+1.1%✅⬆️
.NET Core 3.1 - Baseline
process.internal_duration_ms189.28 ± (188.77 - 189.79) ms189.03 ± (188.60 - 189.46) ms-0.1%✅
process.time_to_main_ms81.20 ± (80.91 - 81.48) ms80.96 ± (80.77 - 81.14) ms-0.3%✅
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%✅
runtime.dotnet.mem.committed16.11 ± (16.08 - 16.13) MB16.07 ± (16.05 - 16.09) MB-0.2%✅
runtime.dotnet.threads.count20 ± (20 - 20)20 ± (19 - 20)-1.1%✅
.NET Core 3.1 - Bailout
process.internal_duration_ms188.27 ± (187.95 - 188.60) ms189.00 ± (188.55 - 189.46) ms+0.4%✅⬆️
process.time_to_main_ms82.04 ± (81.89 - 82.19) ms82.46 ± (82.22 - 82.71) ms+0.5%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%✅
runtime.dotnet.mem.committed16.18 ± (16.15 - 16.21) MB16.17 ± (16.15 - 16.20) MB-0.1%✅
runtime.dotnet.threads.count21 ± (21 - 21)21 ± (20 - 21)-0.4%✅
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms395.22 ± (392.70 - 397.73) ms398.15 ± (395.48 - 400.82) ms+0.7%✅⬆️
process.time_to_main_ms517.06 ± (516.38 - 517.74) ms521.11 ± (520.09 - 522.12) ms+0.8%✅⬆️
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%✅
runtime.dotnet.mem.committed62.65 ± (62.50 - 62.81) MB62.77 ± (62.62 - 62.92) MB+0.2%✅⬆️
runtime.dotnet.threads.count29 ± (29 - 29)29 ± (29 - 29)+0.1%✅⬆️
.NET 6 - Baseline
process.internal_duration_ms199.16 ± (198.70 - 199.61) ms193.38 ± (192.94 - 193.82) ms-2.9%✅
process.time_to_main_ms72.35 ± (72.15 - 72.55) ms70.26 ± (70.04 - 70.49) ms-2.9%✅
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%✅
runtime.dotnet.mem.committed16.24 ± (16.21 - 16.26) MB16.11 ± (15.98 - 16.24) MB-0.8%✅
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (19 - 19)-1.4%✅
.NET 6 - Bailout
process.internal_duration_ms196.07 ± (195.60 - 196.55) ms192.11 ± (191.77 - 192.44) ms-2.0%✅
process.time_to_main_ms72.41 ± (72.21 - 72.61) ms71.02 ± (70.90 - 71.13) ms-1.9%✅
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%✅
runtime.dotnet.mem.committed16.33 ± (16.30 - 16.35) MB16.30 ± (16.20 - 16.39) MB-0.2%✅
runtime.dotnet.threads.count20 ± (20 - 20)20 ± (20 - 20)-1.8%✅
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms417.74 ± (414.56 - 420.93) ms418.93 ± (415.75 - 422.11) ms+0.3%✅⬆️
process.time_to_main_ms490.03 ± (489.34 - 490.73) ms485.95 ± (484.98 - 486.91) ms-0.8%✅
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%✅
runtime.dotnet.mem.committed62.31 ± (62.18 - 62.45) MB62.16 ± (62.03 - 62.30) MB-0.2%✅
runtime.dotnet.threads.count29 ± (29 - 30)30 ± (29 - 30)+0.1%✅⬆️
.NET 8 - Baseline
process.internal_duration_ms190.88 ± (190.49 - 191.27) ms190.33 ± (189.99 - 190.68) ms-0.3%✅
process.time_to_main_ms69.32 ± (69.11 - 69.53) ms69.54 ± (69.33 - 69.75) ms+0.3%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%✅
runtime.dotnet.mem.committed11.73 ± (11.70 - 11.76) MB11.74 ± (11.71 - 11.77) MB+0.1%✅⬆️
runtime.dotnet.threads.count18 ± (18 - 18)18 ± (18 - 18)-0.6%✅
.NET 8 - Bailout
process.internal_duration_ms188.72 ± (188.49 - 188.95) ms190.15 ± (189.82 - 190.48) ms+0.8%✅⬆️
process.time_to_main_ms70.01 ± (69.91 - 70.10) ms70.30 ± (70.18 - 70.41) ms+0.4%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%✅
runtime.dotnet.mem.committed11.70 ± (11.62 - 11.78) MB11.78 ± (11.76 - 11.81) MB+0.7%✅⬆️
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (19 - 19)+0.9%✅⬆️
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms363.28 ± (361.74 - 364.82) ms366.94 ± (365.54 - 368.33) ms+1.0%✅⬆️
process.time_to_main_ms465.13 ± (464.52 - 465.74) ms466.80 ± (465.93 - 467.68) ms+0.4%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%✅
runtime.dotnet.mem.committed50.27 ± (50.23 - 50.31) MB50.31 ± (50.28 - 50.35) MB+0.1%✅⬆️
runtime.dotnet.threads.count29 ± (29 - 29)29 ± (29 - 29)-0.4%✅
Comparison explanation

Execution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

Duration charts
FakeDbCommand (.NET Framework 4.8) 
gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (77ms)  : 68, 86
    master - mean (75ms)  : 71, 79

    section Bailout
    This PR (7838) - mean (81ms)  : 74, 89
    master - mean (79ms)  : 74, 84

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (1,150ms)  : 1048, 1253
    master - mean (1,133ms)  : 1053, 1212
Loading
FakeDbCommand (.NET Core 3.1) 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (118ms)  : 109, 127
    master - mean (118ms)  : 112, 125

    section Bailout
    This PR (7838) - mean (119ms)  : 110, 128
    master - mean (121ms)  : 113, 128

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (812ms)  : 774, 851
    master - mean (814ms)  : 781, 848
Loading
FakeDbCommand (.NET 6) 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (105ms)  : 97, 112
    master - mean (108ms)  : 102, 114

    section Bailout
    This PR (7838) - mean (106ms)  : 98, 115
    master - mean (106ms)  : 100, 112

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (756ms)  : 709, 802
    master - mean (757ms)  : 721, 792
Loading
FakeDbCommand (.NET 8) 
gantt
    title Execution time (ms) FakeDbCommand (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (104ms)  : 97, 110
    master - mean (104ms)  : 99, 109

    section Bailout
    This PR (7838) - mean (105ms)  : 96, 115
    master - mean (105ms)  : 99, 111

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (718ms)  : 687, 750
    master - mean (725ms)  : 694, 757
Loading
HttpMessageHandler (.NET Framework 4.8) 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (200ms)  : 193, 208
    master - mean (192ms)  : 188, 196

    section Bailout
    This PR (7838) - mean (201ms)  : 197, 205
    master - mean (195ms)  : 193, 197

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (1,187ms)  : 1112, 1262
    master - mean (1,175ms)  : 1102, 1248
Loading
HttpMessageHandler (.NET Core 3.1) 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (278ms)  : 271, 286
    master - mean (279ms)  : 271, 287

    section Bailout
    This PR (7838) - mean (280ms)  : 273, 286
    master - mean (278ms)  : 274, 283

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (954ms)  : 913, 995
    master - mean (944ms)  : 898, 991
Loading
HttpMessageHandler (.NET 6) 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (272ms)  : 266, 279
    master - mean (280ms)  : 272, 289

    section Bailout
    This PR (7838) - mean (271ms)  : 267, 275
    master - mean (277ms)  : 269, 284

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (937ms)  : 877, 996
    master - mean (937ms)  : 888, 987
Loading
HttpMessageHandler (.NET 8) 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7838) - mean (270ms)  : 264, 275
    master - mean (270ms)  : 263, 277

    section Bailout
    This PR (7838) - mean (270ms)  : 265, 275
    master - mean (268ms)  : 265, 271

    section CallTarget+Inlining+NGEN
    This PR (7838) - mean (867ms)  : 838, 895
    master - mean (859ms)  : 840, 878
Loading

@datadog-official

This comment has been minimized.

Sign in to view
@pr-commenter
Copy link

pr-commenter bot commented Nov 18, 2025 •
edited
Loading

Benchmarks

Benchmarks Report for benchmark platform 🐌

Benchmarks for #7838 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.145
  • 2 benchmarks are slower, with geometric mean 1.649
  • 4 benchmarks have fewer allocations
  • 6 benchmarks have more allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #7838

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.ActivityBenchmark.StartStopWithChild‑net472 6.03 KB 6.06 KB 31 B 0.51%

Fewer allocations 🎉 in #7838

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.ActivityBenchmark.StartStopWithChild‑net6.0 5.53 KB 5.5 KB -29 B -0.52%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 11.1μs 58.6ns 293ns 0 0 0 5.53 KB
master StartStopWithChild netcoreapp3.1 14.1μs 72.7ns 341ns 0 0 0 5.7 KB
master StartStopWithChild net472 22.8μs 123ns 717ns 1 0.335 0.112 6.03 KB
#7838 StartStopWithChild net6.0 10.3μs 57.1ns 370ns 0 0 0 5.5 KB
#7838 StartStopWithChild netcoreapp3.1 13.8μs 64.7ns 259ns 0 0 0 5.71 KB
#7838 StartStopWithChild net472 22μs 123ns 855ns 0.996 0.332 0.111 6.06 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 943μs 143ns 553ns 0 0 0 2.71 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 1.02ms 263ns 1.02μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 1.23ms 752ns 2.91μs 0 0 0 3.31 KB
#7838 WriteAndFlushEnrichedTraces net6.0 927μs 43.7ns 164ns 0 0 0 2.7 KB
#7838 WriteAndFlushEnrichedTraces netcoreapp3.1 1.04ms 579ns 2.24μs 0 0 0 2.7 KB
#7838 WriteAndFlushEnrichedTraces net472 1.2ms 650ns 2.34μs 0 0 0 3.31 KB
Benchmarks.Trace.Asm.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net6.0 1.06μs 5.91ns 36.9ns 0 0 0 1.22 KB
master AllCycleSimpleBody netcoreapp3.1 1.4μs 7.34ns 35.2ns 0 0 0 1.2 KB
master AllCycleSimpleBody net472 1.04μs 0.214ns 0.773ns 0.193 0 0 1.23 KB
master AllCycleMoreComplexBody net6.0 7μs 32.1ns 129ns 0 0 0 4.72 KB
master AllCycleMoreComplexBody netcoreapp3.1 9μs 33.8ns 126ns 0 0 0 4.62 KB
master AllCycleMoreComplexBody net472 7.65μs 2.35ns 8.8ns 0.727 0 0 4.74 KB
master ObjectExtractorSimpleBody net6.0 319ns 1.59ns 6.57ns 0 0 0 280 B
master ObjectExtractorSimpleBody netcoreapp3.1 394ns 1.95ns 8.06ns 0 0 0 272 B
master ObjectExtractorSimpleBody net472 294ns 0.0443ns 0.172ns 0.0444 0 0 281 B
master ObjectExtractorMoreComplexBody net6.0 6.24μs 33.3ns 173ns 0 0 0 3.78 KB
master ObjectExtractorMoreComplexBody netcoreapp3.1 7.79μs 36.9ns 157ns 0 0 0 3.69 KB
master ObjectExtractorMoreComplexBody net472 6.74μs 7.45ns 28.9ns 0.572 0 0 3.8 KB
#7838 AllCycleSimpleBody net6.0 1.06μs 5.6ns 29.6ns 0 0 0 1.22 KB
#7838 AllCycleSimpleBody netcoreapp3.1 1.45μs 1.19ns 4.45ns 0 0 0 1.2 KB
#7838 AllCycleSimpleBody net472 1.03μs 0.348ns 1.35ns 0.194 0 0 1.23 KB
#7838 AllCycleMoreComplexBody net6.0 7.12μs 37.2ns 134ns 0 0 0 4.72 KB
#7838 AllCycleMoreComplexBody netcoreapp3.1 8.89μs 33.9ns 131ns 0 0 0 4.62 KB
#7838 AllCycleMoreComplexBody net472 7.61μs 7.67ns 29.7ns 0.721 0 0 4.74 KB
#7838 ObjectExtractorSimpleBody net6.0 332ns 1.82ns 10.9ns 0 0 0 280 B
#7838 ObjectExtractorSimpleBody netcoreapp3.1 400ns 2.04ns 8.14ns 0 0 0 272 B
#7838 ObjectExtractorSimpleBody net472 305ns 0.0121ns 0.0438ns 0.0445 0 0 281 B
#7838 ObjectExtractorMoreComplexBody net6.0 6.24μs 29.3ns 113ns 0 0 0 3.78 KB
#7838 ObjectExtractorMoreComplexBody netcoreapp3.1 7.78μs 39.7ns 154ns 0 0 0 3.69 KB
#7838 ObjectExtractorMoreComplexBody net472 6.73μs 1.18ns 4.42ns 0.572 0 0 3.8 KB
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EncodeArgs net6.0 76.4μs 78.4ns 303ns 0 0 0 32.4 KB
master EncodeArgs netcoreapp3.1 96.8μs 297ns 1.15μs 0 0 0 32.4 KB
master EncodeArgs net472 111μs 14.2ns 53.3ns 5.01 0 0 32.5 KB
master EncodeLegacyArgs net6.0 145μs 151ns 587ns 0 0 0 2.15 KB
master EncodeLegacyArgs netcoreapp3.1 195μs 197ns 763ns 0 0 0 2.14 KB
master EncodeLegacyArgs net472 265μs 20ns 77.5ns 0 0 0 2.16 KB
#7838 EncodeArgs net6.0 75.8μs 278ns 1.04μs 0 0 0 32.4 KB
#7838 EncodeArgs netcoreapp3.1 96.5μs 377ns 1.46μs 0 0 0 32.4 KB
#7838 EncodeArgs net472 110μs 10.8ns 42ns 4.94 0 0 32.51 KB
#7838 EncodeLegacyArgs net6.0 148μs 246ns 952ns 0 0 0 2.15 KB
#7838 EncodeLegacyArgs netcoreapp3.1 197μs 226ns 877ns 0 0 0 2.14 KB
#7838 EncodeLegacyArgs net472 264μs 33.7ns 126ns 0 0 0 2.16 KB
Benchmarks.Trace.Asm.AppSecWafBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #7838

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmarkWithAttack‑netcoreapp3.1 2.445 301,427.21 737,132.09

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunWafRealisticBenchmark net6.0 390μs 67.1ns 242ns 0 0 0 4.55 KB
master RunWafRealisticBenchmark netcoreapp3.1 413μs 661ns 2.56μs 0 0 0 4.48 KB
master RunWafRealisticBenchmark net472 428μs 51.2ns 198ns 0 0 0 4.66 KB
master RunWafRealisticBenchmarkWithAttack net6.0 285μs 77.7ns 301ns 0 0 0 2.24 KB
master RunWafRealisticBenchmarkWithAttack netcoreapp3.1 302μs 168ns 581ns 0 0 0 2.22 KB
master RunWafRealisticBenchmarkWithAttack net472 311μs 24.6ns 95.2ns 0 0 0 2.29 KB
#7838 RunWafRealisticBenchmark net6.0 396μs 75.6ns 283ns 0 0 0 4.55 KB
#7838 RunWafRealisticBenchmark netcoreapp3.1 416μs 81.8ns 306ns 0 0 0 4.48 KB
#7838 RunWafRealisticBenchmark net472 431μs 69.6ns 269ns 0 0 0 4.68 KB
#7838 RunWafRealisticBenchmarkWithAttack net6.0 283μs 39.2ns 152ns 0 0 0 2.24 KB
#7838 RunWafRealisticBenchmarkWithAttack netcoreapp3.1 671μs 14.1μs 141μs 0 0 0 2.22 KB
#7838 RunWafRealisticBenchmarkWithAttack net472 310μs 143ns 554ns 0 0 0 2.29 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 60.8μs 81.4ns 315ns 0 0 0 14.58 KB
master SendRequest netcoreapp3.1 71.5μs 102ns 395ns 0 0 0 17.42 KB
master SendRequest net472 0.00854ns 0.00243ns 0.00943ns 0 0 0 0 b
#7838 SendRequest net6.0 60.6μs 93.4ns 337ns 0 0 0 14.52 KB
#7838 SendRequest netcoreapp3.1 71.4μs 349ns 1.4μs 0 0 0 17.42 KB
#7838 SendRequest net472 0.014ns 0.00302ns 0.0117ns 0 0 0 0 b
Benchmarks.Trace.CharSliceBenchmark - Slower ⚠️ More allocations ⚠️

Slower ⚠️ in #7838

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0 1.112 1,344,390.62 1,494,760.07

More allocations ⚠️ in #7838

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0 2 B 4 B 2 B 100.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0 4 B 7 B 3 B 75.00%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master OriginalCharSlice net6.0 1.9ms 4.85μs 18.1μs 0 0 0 640 KB
master OriginalCharSlice netcoreapp3.1 2.11ms 7.29μs 27.3μs 0 0 0 640 KB
master OriginalCharSlice net472 2.66ms 146ns 526ns 100 0 0 641.95 KB
master OptimizedCharSlice net6.0 1.34ms 240ns 898ns 0 0 0 4 B
master OptimizedCharSlice netcoreapp3.1 1.73ms 2.16μs 8.38μs 0 0 0 1 B
master OptimizedCharSlice net472 1.93ms 1.78μs 6.91μs 0 0 0 0 b
master OptimizedCharSliceWithPool net6.0 801μs 77.9ns 302ns 0 0 0 2 B
master OptimizedCharSliceWithPool netcoreapp3.1 852μs 232ns 900ns 0 0 0 0 b
master OptimizedCharSliceWithPool net472 1.14ms 87.8ns 329ns 0 0 0 0 b
#7838 OriginalCharSlice net6.0 1.93ms 704ns 2.73μs 0 0 0 640.01 KB
#7838 OriginalCharSlice netcoreapp3.1 2.14ms 5.55μs 20.8μs 0 0 0 640 KB
#7838 OriginalCharSlice net472 2.66ms 118ns 426ns 100 0 0 641.95 KB
#7838 OptimizedCharSlice net6.0 1.49ms 200ns 776ns 0 0 0 7 B
#7838 OptimizedCharSlice netcoreapp3.1 1.71ms 191ns 738ns 0 0 0 1 B
#7838 OptimizedCharSlice net472 1.97ms 272ns 1.05μs 0 0 0 0 b
#7838 OptimizedCharSliceWithPool net6.0 801μs 97.1ns 376ns 0 0 0 4 B
#7838 OptimizedCharSliceWithPool netcoreapp3.1 840μs 144ns 556ns 0 0 0 0 b
#7838 OptimizedCharSliceWithPool net472 1.14ms 104ns 402ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Faster 🎉 More allocations ⚠️

Faster 🎉 in #7838

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 1.145 964,420.98 842,134.58

More allocations ⚠️ in #7838

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 41.78 KB 42.21 KB 436 B 1.04%

Fewer allocations 🎉 in #7838

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 56.47 KB 56.15 KB -317 B -0.56%
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 42.2 KB 41.73 KB -471 B -1.12%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 629μs 1.76μs 6.33μs 0 0 0 42.2 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 719μs 4.14μs 31.5μs 0 0 0 41.78 KB
master WriteAndFlushEnrichedTraces net472 964μs 3.39μs 13.1μs 8.93 0 0 56.47 KB
#7838 WriteAndFlushEnrichedTraces net6.0 642μs 3.37μs 17.5μs 0 0 0 41.73 KB
#7838 WriteAndFlushEnrichedTraces netcoreapp3.1 734μs 620ns 2.4μs 0 0 0 42.21 KB
#7838 WriteAndFlushEnrichedTraces net472 844μs 2.01μs 7.77μs 8.33 0 0 56.15 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.91μs 7.99ns 30.9ns 0 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 2.7μs 7.56ns 27.3ns 0 0 0 1.02 KB
master ExecuteNonQuery net472 2.83μs 1.7ns 6.38ns 0.156 0.0142 0 987 B
#7838 ExecuteNonQuery net6.0 1.91μs 5.83ns 22.6ns 0 0 0 1.02 KB
#7838 ExecuteNonQuery netcoreapp3.1 2.64μs 10.8ns 40.2ns 0 0 0 1.02 KB
#7838 ExecuteNonQuery net472 2.81μs 3.83ns 14.8ns 0.153 0.0139 0 987 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.72μs 0.689ns 2.58ns 0 0 0 1.03 KB
master CallElasticsearch netcoreapp3.1 2.31μs 10.6ns 41.1ns 0 0 0 1.03 KB
master CallElasticsearch net472 3.59μs 2.37ns 9.17ns 0.16 0 0 1.04 KB
master CallElasticsearchAsync net6.0 1.81μs 7ns 27.1ns 0 0 0 1.01 KB
master CallElasticsearchAsync netcoreapp3.1 2.41μs 8.22ns 30.7ns 0 0 0 1.08 KB
master CallElasticsearchAsync net472 3.72μs 5.41ns 21ns 0.167 0 0 1.1 KB
#7838 CallElasticsearch net6.0 1.67μs 8.46ns 37.8ns 0 0 0 1.03 KB
#7838 CallElasticsearch netcoreapp3.1 2.22μs 7.83ns 30.3ns 0 0 0 1.03 KB
#7838 CallElasticsearch net472 3.48μs 1.46ns 5.45ns 0.157 0 0 1.04 KB
#7838 CallElasticsearchAsync net6.0 1.92μs 9.57ns 39.5ns 0 0 0 1.01 KB
#7838 CallElasticsearchAsync netcoreapp3.1 2.34μs 9.94ns 35.9ns 0 0 0 1.08 KB
#7838 CallElasticsearchAsync net472 3.61μs 3.45ns 13.4ns 0.163 0 0 1.1 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.91μs 6.61ns 25.6ns 0 0 0 952 B
master ExecuteAsync netcoreapp3.1 2.44μs 8.59ns 33.3ns 0 0 0 952 B
master ExecuteAsync net472 2.61μs 1.24ns 4.3ns 0.144 0 0 915 B
#7838 ExecuteAsync net6.0 1.8μs 7.76ns 30.1ns 0 0 0 952 B
#7838 ExecuteAsync netcoreapp3.1 2.45μs 7.25ns 28.1ns 0 0 0 952 B
#7838 ExecuteAsync net472 2.61μs 5.87ns 22.7ns 0.144 0 0 915 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 7.01μs 24.1ns 93.3ns 0 0 0 2.36 KB
master SendAsync netcoreapp3.1 8.55μs 12.7ns 47.6ns 0 0 0 2.9 KB
master SendAsync net472 12.3μs 8.09ns 30.3ns 0.492 0 0 3.18 KB
#7838 SendAsync net6.0 6.89μs 6.89ns 25.8ns 0 0 0 2.36 KB
#7838 SendAsync netcoreapp3.1 8.67μs 17.5ns 67.6ns 0 0 0 2.9 KB
#7838 SendAsync net472 12.1μs 6.77ns 25.3ns 0.478 0 0 3.18 KB
Benchmarks.Trace.Iast.StringAspectsBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #7838

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 257.14 KB 277.31 KB 20.17 KB 7.84%
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑netcoreapp3.1 42.73 KB 45.5 KB 2.78 KB 6.50%

Fewer allocations 🎉 in #7838

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark‑net6.0 44.1 KB 43.71 KB -392 B -0.89%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StringConcatBenchmark net6.0 44μs 237ns 1.28μs 0 0 0 44.1 KB
master StringConcatBenchmark netcoreapp3.1 49.6μs 289ns 2.33μs 0 0 0 42.73 KB
master StringConcatBenchmark net472 57.1μs 251ns 973ns 0 0 0 57.34 KB
master StringConcatAspectBenchmark net6.0 481μs 1.93μs 8.84μs 0 0 0 257.14 KB
master StringConcatAspectBenchmark netcoreapp3.1 553μs 2.35μs 9.1μs 0 0 0 276.95 KB
master StringConcatAspectBenchmark net472 409μs 2.19μs 11.2μs 0 0 0 278.53 KB
#7838 StringConcatBenchmark net6.0 42.2μs 222ns 1.13μs 0 0 0 43.71 KB
#7838 StringConcatBenchmark netcoreapp3.1 50.8μs 332ns 3.13μs 0 0 0 45.5 KB
#7838 StringConcatBenchmark net472 57.6μs 280ns 1.12μs 0 0 0 57.34 KB
#7838 StringConcatAspectBenchmark net6.0 471μs 1.98μs 7.93μs 0 0 0 277.31 KB
#7838 StringConcatAspectBenchmark netcoreapp3.1 528μs 1.29μs 4.46μs 0 0 0 276.98 KB
#7838 StringConcatAspectBenchmark net472 408μs 2.25μs 13.5μs 0 0 0 278.53 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.6μs 12.7ns 53.7ns 0 0 0 1.7 KB
master EnrichedLog netcoreapp3.1 3.56μs 16.7ns 64.9ns 0 0 0 1.7 KB
master EnrichedLog net472 3.91μs 4.49ns 17.4ns 0.253 0 0 1.64 KB
#7838 EnrichedLog net6.0 2.64μs 12.8ns 49.4ns 0 0 0 1.7 KB
#7838 EnrichedLog netcoreapp3.1 3.66μs 17.9ns 71.6ns 0 0 0 1.7 KB
#7838 EnrichedLog net472 3.82μs 3.11ns 12ns 0.248 0 0 1.64 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 121μs 55ns 198ns 0 0 0 4.31 KB
master EnrichedLog netcoreapp3.1 126μs 38.9ns 135ns 0 0 0 4.31 KB
master EnrichedLog net472 166μs 31.1ns 112ns 0 0 0 4.52 KB
#7838 EnrichedLog net6.0 124μs 76.5ns 265ns 0 0 0 4.31 KB
#7838 EnrichedLog netcoreapp3.1 127μs 156ns 582ns 0 0 0 4.31 KB
#7838 EnrichedLog net472 167μs 59ns 229ns 0 0 0 4.52 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 5.01μs 2.82ns 10.9ns 0 0 0 2.26 KB
master EnrichedLog netcoreapp3.1 6.75μs 14.7ns 55.2ns 0 0 0 2.26 KB
master EnrichedLog net472 7.56μs 5.86ns 22.7ns 0.302 0 0 2.08 KB
#7838 EnrichedLog net6.0 4.98μs 19ns 71ns 0 0 0 2.26 KB
#7838 EnrichedLog netcoreapp3.1 6.99μs 11.5ns 44.5ns 0 0 0 2.26 KB
#7838 EnrichedLog net472 7.46μs 5.53ns 21.4ns 0.297 0 0 2.08 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.98μs 10.2ns 50.8ns 0 0 0 1.2 KB
master SendReceive netcoreapp3.1 2.65μs 11.1ns 42.9ns 0 0 0 1.2 KB
master SendReceive net472 3.21μs 5.15ns 19.9ns 0.176 0 0 1.2 KB
#7838 SendReceive net6.0 2.07μs 2.14ns 8.3ns 0 0 0 1.2 KB
#7838 SendReceive netcoreapp3.1 2.59μs 10.2ns 39.3ns 0 0 0 1.2 KB
#7838 SendReceive net472 3.09μs 2.7ns 10.4ns 0.184 0 0 1.2 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 4.21μs 2.82ns 10.5ns 0 0 0 1.58 KB
master EnrichedLog netcoreapp3.1 5.65μs 14.4ns 55.9ns 0 0 0 1.63 KB
master EnrichedLog net472 6.44μs 4.44ns 16ns 0.32 0 0 2.03 KB
#7838 EnrichedLog net6.0 4.33μs 1.99ns 7.44ns 0 0 0 1.58 KB
#7838 EnrichedLog netcoreapp3.1 5.58μs 17.4ns 67.4ns 0 0 0 1.63 KB
#7838 EnrichedLog net472 6.68μs 6.12ns 23.7ns 0.299 0 0 2.03 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 761ns 3.94ns 19.7ns 0 0 0 576 B
master StartFinishSpan netcoreapp3.1 978ns 0.909ns 3.52ns 0 0 0 576 B
master StartFinishSpan net472 978ns 0.335ns 1.21ns 0.0882 0 0 578 B
master StartFinishScope net6.0 908ns 4.68ns 22.4ns 0 0 0 696 B
master StartFinishScope netcoreapp3.1 1.18μs 5.86ns 25.6ns 0 0 0 696 B
master StartFinishScope net472 1.19μs 0.243ns 0.875ns 0.0995 0 0 658 B
#7838 StartFinishSpan net6.0 776ns 4.03ns 20.6ns 0 0 0 576 B
#7838 StartFinishSpan netcoreapp3.1 983ns 5.26ns 28.8ns 0 0 0 576 B
#7838 StartFinishSpan net472 952ns 0.29ns 1.12ns 0.0908 0 0 578 B
#7838 StartFinishScope net6.0 918ns 1.77ns 6.85ns 0 0 0 696 B
#7838 StartFinishScope netcoreapp3.1 1.17μs 5.87ns 27.5ns 0 0 0 696 B
#7838 StartFinishScope net472 1.19μs 0.657ns 2.54ns 0.101 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 1.02μs 5.68ns 33.6ns 0 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 1.44μs 5.48ns 21.2ns 0 0 0 696 B
master RunOnMethodBegin net472 1.56μs 0.988ns 3.83ns 0.101 0 0 658 B
#7838 RunOnMethodBegin net6.0 1.04μs 5.46ns 31.4ns 0 0 0 696 B
#7838 RunOnMethodBegin netcoreapp3.1 1.4μs 7.03ns 31.5ns 0 0 0 696 B
#7838 RunOnMethodBegin net472 1.46μs 2.29ns 8.89ns 0.102 0 0 658 B

@NachoEchevarria NachoEchevarria changed the title Fix racing conditions Fix racing conditions in Dataflow Nov 18, 2025
@NachoEchevarria NachoEchevarria changed the title Fix racing conditions in Dataflow [IAST] Fix racing conditions in Dataflow Nov 19, 2025
@NachoEchevarria NachoEchevarria marked this pull request as ready for review November 19, 2025 11:41
@NachoEchevarria NachoEchevarria requested a review from a team as a code owner November 19, 2025 11:41
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Sign in to view

@NachoEchevarria NachoEchevarria added area:tests unit tests, integration tests area:asm labels Nov 19, 2025
andrewlock
andrewlock reviewed Nov 19, 2025
View reviewed changes
tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
if (!IsInitialized())
{
_initialized = true;
SetInitialized(true);
Copy link
Member

@andrewlock andrewlock Nov 19, 2025

Choose a reason for hiding this comment

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

Silly question, but could this also be part of the problem? 🤔

We set initialized = true here before we've finished the initialization. That means that all the other places where we do if (!IsInitialized()) and bail out will stop bailing 🤔 Also, we don't have a CSGUARD in here, should we?

I feel like we probably need something more like this?

CSGUARD(_cs);
if (!IsInitialized())
{
    // Do all the other work in the if block

    SetInitialized(true);
}

Copy link
Member

@tonyredondo tonyredondo Nov 19, 2025

Choose a reason for hiding this comment

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

yes I do think you need to guard in this one as well.

Copy link
Contributor

@daniel-romano-DD daniel-romano-DD Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The main problem is here, in LoadAspects, and this function is the one that has to be well covered. _isInitialized should be set to true as the last thing done here, not in the begining of the function.

tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
HRESULT Dataflow::AppDomainShutdown(AppDomainID appDomainId)
{
if (!_initialized)
CSGUARD(_cs);
Copy link
Member

@andrewlock andrewlock Nov 19, 2025

Choose a reason for hiding this comment

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

Do we want to use double checking here to avoid contention? I'm not sure it's a big problem for AppDomainShutdown really, and how often would we not be initialized? 🤔

// Initial check outside the lock
if (!IsInitialized())
{
    return S_OK;
}

CSGUARD(_cs);
// Double check inside the lock
if (!IsInitialized())
{
    return S_OK;
}

Copy link
Member

@tonyredondo tonyredondo Nov 19, 2025

Choose a reason for hiding this comment

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

If we add the guard for each IsInitialized() call we should consider to just use a normal bool instead of the atomic bool.

Copy link
Contributor

@daniel-romano-DD daniel-romano-DD Nov 19, 2025

Choose a reason for hiding this comment

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

We are not going to find the situation where a threar reads true while it has been set to false. The problem is the oposite, in initialization. IMO the whole thing could be solved with deferring the set to true in _isInitialized to the end of the LoadAspects function.

tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
HRESULT Dataflow::ModuleLoaded(ModuleID moduleId, ModuleInfo** pModuleInfo)
{
if (!_initialized)
if (!IsInitialized())
Copy link
Member

@andrewlock andrewlock Nov 19, 2025

Choose a reason for hiding this comment

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

This still has the CSGUARD after the IsInitialized() check (because it takes the lock in GetModuleInfo... should we move this check inside GetModuleInfo() instead?

Copy link
Collaborator Author

@NachoEchevarria NachoEchevarria Nov 19, 2025

Choose a reason for hiding this comment

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

GetModuleInfo() already checks for that, this is just an early check.

tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
bool Dataflow::IsInlineEnabled(ModuleID calleeModuleId, mdToken calleeMethodId)
{
if (!_initialized)
if (!IsInitialized())
Copy link
Member

@andrewlock andrewlock Nov 19, 2025

Choose a reason for hiding this comment

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

Seems like there's a bunch of places here without the guards... is that safe?

Copy link
Collaborator Author

@NachoEchevarria NachoEchevarria Nov 19, 2025

Choose a reason for hiding this comment

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

It is safe as long as we dont perform sensitive operations. In this case, IsInitialized() is already safe because we are using an atomic bool and we will recheck that later in a locked envirinment.
Methods like IsInlineEnabled(), JITProcessMethod(), and JITCompilationStarted() check IsInitialized() outside locks because the atomic read itself is safe, they perform no "sensitive operations" (directly accessing shared state _modules, _appDomains, _moduleAspects) and the real protection is at the leaf level. When these methods call GetModuleInfo() or GetAppDomain(), those methods acquire the lock first, check IsInitialized() inside the lock (TOCTOU-safe) and only then access shared maps.

gleocadie
gleocadie approved these changes Nov 19, 2025
View reviewed changes
Copy link
Collaborator

@gleocadie gleocadie left a comment

Choose a reason for hiding this comment

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

LGTM

daniel-romano-DD
daniel-romano-DD reviewed Nov 19, 2025
View reviewed changes
tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
Dataflow::~Dataflow()
{
_initialized = false;
CSGUARD(_cs);
Copy link
Contributor

@daniel-romano-DD daniel-romano-DD Nov 19, 2025

Choose a reason for hiding this comment

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

Dataflow is destroyed on profiler unload and this is the only place where _initialized is set to false. This CS is unnecessary (although does not hurt).

daniel-romano-DD
daniel-romano-DD requested changes Nov 19, 2025
View reviewed changes
Copy link
Contributor

@daniel-romano-DD daniel-romano-DD left a comment

Choose a reason for hiding this comment

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

This is overkill for the real crash reason. A calmer look and a different approach is needed.

@daniel-romano-DD daniel-romano-DD mentioned this pull request Nov 23, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonyredondo tonyredondo tonyredondo left review comments

@andrewlock andrewlock andrewlock left review comments

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@daniel-romano-DD daniel-romano-DD daniel-romano-DD requested changes

@gleocadie gleocadie gleocadie approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

area:asm area:asm-iast area:tests unit tests, integration tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@NachoEchevarria @tonyredondo @gleocadie @andrewlock @daniel-romano-DD