Skip to content

Commit c43abbb

Browse files
Simplify dataflow synchronization (#7853)
## Summary of changes This PR simplifies the lifecycle and usage of iast::Dataflow within the .NET tracer by removing unnecessary synchronization complexity and consolidating initialization logic. After validating that the profiler’s lifecycle guarantees make the race condition targeted in [#7838](#7838) impossible, the extra locking and state checks introduced there were identified as unnecessary. This PR therefore removes them and replaces them with a clearer, more maintainable initialization model. The end result is the same runtime behavior, but with cleaner, more predictable code paths. ## Reason for change During investigation of the suspected race in Dataflow, we discovered: _dataflow is created before any IAST-related callbacks can run. _dataflow is destroyed only during CorProfiler::Shutdown, after all runtime callbacks have stopped. Therefore, there is no window where runtime threads can access a partially-initialized or null Dataflow instance. Because the profiler’s lifecycle already prevents the problematic interleavings, the additional locking and defensive checks from #7838 were redundant. They increased complexity without improving correctness. This PR cleans that up. ## Implementation details What this PR does 1. Consolidates Dataflow initialization logic Dataflow::LoadAspects becomes the single, authoritative entry point for initialization. No scattered flags, no duplicated checks. 2. Removes unnecessary locking around initialization Since lifecycle guarantees prevent concurrent initialization, the critical-section guarding and initialization state added in #7838 are removed. 3. Simplifies callback paths Profiler callbacks (ModuleLoaded, ModuleUnloaded, IAST instrumentation setup, etc.) now: Assume _dataflow exists. Only guard the internal data structures that are actually accessed concurrently. Remove early-return branches and over-defensive locking patterns. 4. Cleans up dead code and unused members Any state variables, flags, or locking constructs made obsolete by the simplified lifecycle are removed, shrinking the Dataflow class footprint. Result Same behavioral correctness. Fewer code paths and edge cases. Clearer lifecycle contract: Dataflow always exists during the period where callbacks may use it. Easier to maintain and reason about for future IAST development. ## Test coverage Re-ran IAST tests on Windows x86/x64 to confirm no regressions. Validated profiler lifecycle behavior to ensure _dataflow is always valid during runtime callbacks. Confirmed no reappearance of previous intermittent failures. ## Other details --------- Co-authored-by: Andrew Lock <[email protected]>
1 parent 8c4a8c4 commit c43abbb

File tree

3 files changed

+9
-46
lines changed

3 files changed

+9
-46
lines changed

tracer/src/Datadog.Tracer.Native/cor_profiler.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::Initialize(IUnknown* cor_profiler_info_un
255255
return E_FAIL;
256256
}
257257

258-
// CallSite stuff
258+
// Legacy callSite stuff
259259
if (!IsCallSiteManagedActivationEnabled())
260260
{
261261
Logger::Info("Callsite managed activation is disabled.");
@@ -1301,8 +1301,6 @@ HRESULT STDMETHODCALLTYPE CorProfiler::Shutdown()
13011301
rejit_handler = nullptr;
13021302
}
13031303

1304-
DEL(_dataflow);
1305-
13061304
auto definitions = definitions_ids.Get();
13071305

13081306
Logger::Info("Exiting...");
@@ -2072,10 +2070,12 @@ long CorProfiler::DisableCallTargetDefinitions(UINT32 disabledCategories)
20722070
int CorProfiler::RegisterIastAspects(WCHAR** aspects, int aspectsLength, UINT32 enabledCategories, UINT32 platform)
20732071
{
20742072
auto _ = trace::Stats::Instance()->InitializeProfilerMeasure();
2073+
auto definitions = definitions_ids.Get(); // Synchronize Aspects loading
20752074

20762075
auto dataflow = _dataflow;
20772076
if (dataflow == nullptr && IsCallSiteManagedActivationEnabled())
20782077
{
2078+
Logger::Debug("Creating Dataflow.");
20792079
dataflow = new iast::Dataflow(info_, rejit_handler, runtime_information_);
20802080
}
20812081

@@ -2090,6 +2090,7 @@ int CorProfiler::RegisterIastAspects(WCHAR** aspects, int aspectsLength, UINT32
20902090
{
20912091
Logger::Info("Callsite instrumentation is disabled.");
20922092
}
2093+
20932094
return 0;
20942095
}
20952096

tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -182,19 +182,15 @@ Dataflow::Dataflow(ICorProfilerInfo* profiler, std::shared_ptr<RejitHandler> rej
182182

183183
Dataflow::~Dataflow()
184184
{
185-
_initialized = false;
186185
REL(_profiler);
187-
DEL_MAP_VALUES(_modules);
188-
DEL_MAP_VALUES(_appDomains);
189-
DEL_MAP_VALUES(_moduleAspects);
190186
}
191187

192188
void Dataflow::LoadAspects(WCHAR** aspects, int aspectsLength, UINT32 enabledCategories, UINT32 platform)
193189
{
194-
if (!_initialized)
195-
{
196-
_initialized = true;
190+
CSGUARD(_cs);
197191

192+
if (_aspects.size() == 0)
193+
{
198194
// Init aspects
199195
DBG("Dataflow::LoadAspects -> Processing aspects... ", aspectsLength, " Enabled categories: ", enabledCategories, " Platform: ", platform);
200196

@@ -369,11 +365,6 @@ void Dataflow::LoadSecurityControls()
369365

370366
HRESULT Dataflow::AppDomainShutdown(AppDomainID appDomainId)
371367
{
372-
if (!_initialized)
373-
{
374-
return S_OK;
375-
}
376-
377368
CSGUARD(_cs);
378369
auto it = _appDomains.find(appDomainId);
379370
if (it != _appDomains.end())
@@ -388,22 +379,12 @@ HRESULT Dataflow::AppDomainShutdown(AppDomainID appDomainId)
388379

389380
HRESULT Dataflow::ModuleLoaded(ModuleID moduleId, ModuleInfo** pModuleInfo)
390381
{
391-
if (!_initialized)
392-
{
393-
return S_OK;
394-
}
395-
396382
GetModuleInfo(moduleId);
397383
return S_OK;
398384
}
399385

400386
HRESULT Dataflow::ModuleUnloaded(ModuleID moduleId)
401387
{
402-
if (!_initialized)
403-
{
404-
return S_OK;
405-
}
406-
407388
CSGUARD(_cs);
408389
{
409390
auto it = _moduleAspects.find(moduleId);
@@ -608,6 +589,7 @@ ModuleInfo* Dataflow::GetAspectsModule(AppDomainID id)
608589

609590
MethodInfo* Dataflow::GetMethodInfo(ModuleID moduleId, mdMethodDef methodId)
610591
{
592+
CSGUARD(_cs);
611593
auto module = GetModuleInfo(moduleId);
612594
if (module)
613595
{
@@ -618,11 +600,6 @@ MethodInfo* Dataflow::GetMethodInfo(ModuleID moduleId, mdMethodDef methodId)
618600

619601
bool Dataflow::IsInlineEnabled(ModuleID calleeModuleId, mdToken calleeMethodId)
620602
{
621-
if (!_initialized)
622-
{
623-
return true;
624-
}
625-
626603
auto method = JITProcessMethod(calleeModuleId, calleeMethodId);
627604
if (method)
628605
{
@@ -632,21 +609,12 @@ bool Dataflow::IsInlineEnabled(ModuleID calleeModuleId, mdToken calleeMethodId)
632609
}
633610
bool Dataflow::JITCompilationStarted(ModuleID moduleId, mdToken methodId)
634611
{
635-
if (!_initialized)
636-
{
637-
return false;
638-
}
639-
640612
auto method = JITProcessMethod(moduleId, methodId);
641613
return method != nullptr;
642614
}
643615
MethodInfo* Dataflow::JITProcessMethod(ModuleID moduleId, mdToken methodId, trace::FunctionControlWrapper* pFunctionControl)
644616
{
645-
if (!_initialized)
646-
{
647-
return nullptr;
648-
}
649-
617+
CSGUARD(_cs);
650618
MethodInfo* method = nullptr;
651619
auto module = GetModuleInfo(moduleId);
652620
if (module && !module->IsExcluded())
@@ -783,11 +751,6 @@ void Dataflow::AddNGenInlinerModule(ModuleID moduleId)
783751

784752
HRESULT Dataflow::RejitMethod(trace::FunctionControlWrapper& functionControl)
785753
{
786-
if (!_initialized)
787-
{
788-
return S_FALSE;
789-
}
790-
791754
auto method = JITProcessMethod(functionControl.GetModuleId(), functionControl.GetMethodId(), &functionControl);
792755
if (method && method->IsWritten())
793756
{

tracer/src/Datadog.Tracer.Native/iast/dataflow.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ namespace iast
6565

6666
void LoadSecurityControls();
6767
protected:
68-
bool _initialized = false;
6968
bool _setILOnJit = false;
7069

7170
std::vector<DataflowAspectClass*> _aspectClasses;

0 commit comments

Comments
 (0)