Skip to content

Commit d1cda49

Browse files
andrewlockKevin Gosse
andauthored
Revert "Load the tracer/profiler after guardrails checks" (#6200) (#6205)
Reverts DataDog/dd-trace-dotnet#5968 It seems it causes a crash when .NET Framework and .NET Core are loaded in the same process. Cherry pick of #6200 Co-authored-by: Kevin Gosse <[email protected]>
1 parent d0602aa commit d1cda49

File tree

9 files changed

+46
-164
lines changed

9 files changed

+46
-164
lines changed

Datadog.Trace.Native.sln

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Datadog.Trace.ClrProfiler.M
2929
EndProject
3030
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Datadog.Trace.ClrProfiler.Native", "shared\src\Datadog.Trace.ClrProfiler.Native\Datadog.Trace.ClrProfiler.Native.vcxproj", "{48D12C26-6E63-419C-BFE2-E668E8550613}"
3131
EndProject
32-
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Datadog.Trace.ClrProfiler.Native.Tests", "shared\test\Datadog.Trace.ClrProfiler.Native.Tests\Datadog.Trace.ClrProfiler.Native.Tests.vcxproj", "{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}"
33-
EndProject
3432
Global
3533
GlobalSection(SolutionConfigurationPlatforms) = preSolution
3634
Debug|Any CPU = Debug|Any CPU
@@ -178,26 +176,6 @@ Global
178176
{48D12C26-6E63-419C-BFE2-E668E8550613}.Release|x64.Build.0 = Release|x64
179177
{48D12C26-6E63-419C-BFE2-E668E8550613}.Release|x86.ActiveCfg = Release|Win32
180178
{48D12C26-6E63-419C-BFE2-E668E8550613}.Release|x86.Build.0 = Release|Win32
181-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|Any CPU.ActiveCfg = Debug|x64
182-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|Any CPU.Build.0 = Debug|x64
183-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|ARM64.ActiveCfg = Debug|ARM64
184-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|ARM64.Build.0 = Debug|ARM64
185-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|ARM64EC.ActiveCfg = Debug|x64
186-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|ARM64EC.Build.0 = Debug|x64
187-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|x64.ActiveCfg = Debug|x64
188-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|x64.Build.0 = Debug|x64
189-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|x86.ActiveCfg = Debug|Win32
190-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Debug|x86.Build.0 = Debug|Win32
191-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|Any CPU.ActiveCfg = Release|x64
192-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|Any CPU.Build.0 = Release|x64
193-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|ARM64.ActiveCfg = Release|ARM64
194-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|ARM64.Build.0 = Release|ARM64
195-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|ARM64EC.ActiveCfg = Release|x64
196-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|ARM64EC.Build.0 = Release|x64
197-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|x64.ActiveCfg = Release|x64
198-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|x64.Build.0 = Release|x64
199-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|x86.ActiveCfg = Release|Win32
200-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B}.Release|x86.Build.0 = Release|Win32
201179
EndGlobalSection
202180
GlobalSection(SolutionProperties) = preSolution
203181
HideSolutionNode = FALSE
@@ -210,7 +188,6 @@ Global
210188
{6CE95C50-9533-4650-8F11-BCE30908DCDF} = {B9AA20A4-0F9A-47FB-B3BE-A5BDEA42EFF0}
211189
{0686E907-996A-4D6D-A685-D9C0F932C405} = {9E5F0022-0A50-40BF-AC6A-C3078585ECAB}
212190
{48D12C26-6E63-419C-BFE2-E668E8550613} = {9E5F0022-0A50-40BF-AC6A-C3078585ECAB}
213-
{3FCBD2EB-ACBE-4DA1-850F-12BEE08B937B} = {8CEC2042-F11C-49F5-A674-2355793B600A}
214191
EndGlobalSection
215192
GlobalSection(ExtensibilityGlobals) = postSolution
216193
SolutionGuid = {160A1D00-1F5B-40F8-A155-621B4459D78F}

shared/src/Datadog.Trace.ClrProfiler.Native/cor_profiler.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -257,25 +257,13 @@ namespace datadog::shared::nativeloader
257257
}
258258

259259
//
260-
// Initialize the dispatcher
260+
// Get and set profiler pointers
261261
//
262262
if (m_dispatcher == nullptr)
263263
{
264-
Log::Error("Dispatcher is not set.");
265-
single_step_guard_rails.RecordBootstrapError(runtimeInformation, "initialization_error");
266-
return E_FAIL;
267-
}
268-
269-
if (FAILED(m_dispatcher->Initialize()))
270-
{
271-
Log::Error("Error initializing the dispatcher.");
272264
single_step_guard_rails.RecordBootstrapError(runtimeInformation, "initialization_error");
273265
return E_FAIL;
274266
}
275-
276-
//
277-
// Get and set profiler pointers
278-
//
279267
IDynamicInstance* cpInstance = m_dispatcher->GetContinuousProfilerInstance();
280268
if (cpInstance != nullptr)
281269
{

shared/src/Datadog.Trace.ClrProfiler.Native/cor_profiler_class_factory.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,13 @@ HRESULT STDMETHODCALLTYPE CorProfilerClassFactory::QueryInterface(REFIID riid, v
2727
{
2828
*ppvObject = this;
2929
this->AddRef();
30-
30+
31+
// We try to load the class factory of all target cor profilers.
32+
if (FAILED(m_dispatcher->LoadClassFactory(riid)))
33+
{
34+
Log::Warn("Error loading all cor profiler class factories.");
35+
}
36+
3137
return S_OK;
3238
}
3339

@@ -65,7 +71,11 @@ HRESULT STDMETHODCALLTYPE CorProfilerClassFactory::CreateInstance(IUnknown* pUnk
6571

6672
auto profiler = new datadog::shared::nativeloader::CorProfiler(m_dispatcher);
6773
HRESULT res = profiler->QueryInterface(riid, ppvObject);
68-
if (FAILED(res))
74+
if (SUCCEEDED(res))
75+
{
76+
m_dispatcher->LoadInstance(pUnkOuter, riid);
77+
}
78+
else
6979
{
7080
delete profiler;
7181
}

shared/src/Datadog.Trace.ClrProfiler.Native/dllmain.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ EXTERN_C BOOL STDMETHODCALLTYPE DllMain(HMODULE hModule, DWORD ul_reason_for_cal
9494
}
9595
#endif
9696

97-
dispatcher = new DynamicDispatcherImpl();
97+
dispatcher = new DynamicDispatcherImpl();
98+
dispatcher->LoadConfiguration(GetConfigurationFilePath());
9899

99100
// *****************************************************************************************************************
100101
break;

shared/src/Datadog.Trace.ClrProfiler.Native/dynamic_dispatcher.cpp

Lines changed: 20 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -22,43 +22,10 @@ namespace datadog::shared::nativeloader
2222
DynamicDispatcherImpl::DynamicDispatcherImpl() :
2323
m_continuousProfilerInstance(nullptr),
2424
m_tracerInstance(nullptr),
25-
m_customInstance(nullptr),
26-
m_initialized(false),
27-
m_initializationResult(E_UNEXPECTED)
25+
m_customInstance(nullptr)
2826
{
2927
}
3028

31-
HRESULT DynamicDispatcherImpl::Initialize()
32-
{
33-
if (m_initialized)
34-
{
35-
return m_initializationResult;
36-
}
37-
38-
m_initialized = true;
39-
40-
LoadConfiguration(GetConfigurationFilePath());
41-
42-
m_initializationResult = LoadClassFactory(IID_IClassFactory);
43-
44-
if (FAILED(m_initializationResult))
45-
{
46-
Log::Error("Error loading all cor profiler class factories.");
47-
return m_initializationResult;
48-
}
49-
50-
m_initializationResult = LoadInstance();
51-
52-
if (FAILED(m_initializationResult))
53-
{
54-
Log::Error("Error loading all cor profiler instances.");
55-
return m_initializationResult;
56-
}
57-
58-
m_initializationResult = S_OK;
59-
return m_initializationResult;
60-
}
61-
6229
void DynamicDispatcherImpl::LoadConfiguration(fs::path&& configFilePath)
6330
{
6431
::shared::WSTRING nativeLoaderPath = ::shared::GetCurrentModuleFileName();
@@ -190,28 +157,19 @@ namespace datadog::shared::nativeloader
190157

191158
HRESULT DynamicDispatcherImpl::LoadClassFactory(REFIID riid)
192159
{
193-
// We consider the loading a success if at least one class factory is properly loaded.
194-
HRESULT GHR = E_FAIL;
160+
HRESULT GHR = S_OK;
195161

196162
if (m_continuousProfilerInstance != nullptr)
197163
{
198164
HRESULT result = m_continuousProfilerInstance->LoadClassFactory(riid);
199165
if (FAILED(result))
200166
{
201167
Log::Warn("DynamicDispatcherImpl::LoadClassFactory: Error trying to load continuous profiler class factory in: ",
202-
m_continuousProfilerInstance->GetFilePath(), ", error code: ", result);
168+
m_continuousProfilerInstance->GetFilePath());
203169

204170
// If we cannot load the class factory we release the instance.
205171
m_continuousProfilerInstance.release();
206-
207-
if (GHR != S_OK)
208-
{
209-
GHR = result;
210-
}
211-
}
212-
else
213-
{
214-
GHR = S_OK;
172+
GHR = result;
215173
}
216174
}
217175

@@ -220,20 +178,11 @@ namespace datadog::shared::nativeloader
220178
HRESULT result = m_tracerInstance->LoadClassFactory(riid);
221179
if (FAILED(result))
222180
{
223-
Log::Warn("DynamicDispatcherImpl::LoadClassFactory: Error trying to load tracer class factory in: ",
224-
m_tracerInstance->GetFilePath(), ", error code: ", result);
181+
Log::Warn("DynamicDispatcherImpl::LoadClassFactory: Error trying to load tracer class factory in: ", m_tracerInstance->GetFilePath());
225182

226183
// If we cannot load the class factory we release the instance.
227184
m_tracerInstance.release();
228-
229-
if (GHR != S_OK)
230-
{
231-
GHR = result;
232-
}
233-
}
234-
else
235-
{
236-
GHR = S_OK;
185+
GHR = result;
237186
}
238187
}
239188

@@ -242,94 +191,58 @@ namespace datadog::shared::nativeloader
242191
HRESULT result = m_customInstance->LoadClassFactory(riid);
243192
if (FAILED(result))
244193
{
245-
Log::Warn("DynamicDispatcherImpl::LoadClassFactory: Error trying to load custom class factory in: ",
246-
m_customInstance->GetFilePath(), ", error code: ", result);
194+
Log::Warn("DynamicDispatcherImpl::LoadClassFactory: Error trying to load custom class factory in: ", m_customInstance->GetFilePath());
247195

248196
// If we cannot load the class factory we release the instance.
249197
m_customInstance.release();
250-
251-
if (GHR != S_OK)
252-
{
253-
GHR = result;
254-
}
255-
}
256-
else
257-
{
258-
GHR = S_OK;
198+
GHR = result;
259199
}
260200
}
261201

262202
return GHR;
263203
}
264204

265-
HRESULT DynamicDispatcherImpl::LoadInstance()
205+
HRESULT DynamicDispatcherImpl::LoadInstance(IUnknown* pUnkOuter, REFIID riid)
266206
{
267-
// We consider the loading a success if at least one class factory is properly loaded.
268-
HRESULT GHR = E_FAIL;
207+
HRESULT GHR = S_OK;
269208

270209
if (m_continuousProfilerInstance != nullptr)
271210
{
272-
HRESULT result = m_continuousProfilerInstance->LoadInstance();
211+
HRESULT result = m_continuousProfilerInstance->LoadInstance(pUnkOuter, riid);
273212
if (FAILED(result))
274213
{
275214
Log::Warn("DynamicDispatcherImpl::LoadInstance: Error trying to load the continuous profiler instance in: ",
276-
m_continuousProfilerInstance->GetFilePath(), ", error code: ", result);
215+
m_continuousProfilerInstance->GetFilePath());
277216

278217
// If we cannot load the class factory we release the instance.
279218
m_continuousProfilerInstance.release();
280-
281-
if (GHR != S_OK)
282-
{
283-
GHR = result;
284-
}
285-
}
286-
else
287-
{
288-
GHR = S_OK;
219+
GHR = result;
289220
}
290221
}
291222

292223
if (m_tracerInstance != nullptr)
293224
{
294-
HRESULT result = m_tracerInstance->LoadInstance();
225+
HRESULT result = m_tracerInstance->LoadInstance(pUnkOuter, riid);
295226
if (FAILED(result))
296227
{
297-
Log::Warn("DynamicDispatcherImpl::LoadInstance: Error trying to load the tracer instance in: ",
298-
m_tracerInstance->GetFilePath(), ", error code: ", result);
228+
Log::Warn("DynamicDispatcherImpl::LoadInstance: Error trying to load the tracer instance in: ", m_tracerInstance->GetFilePath());
299229

300230
// If we cannot load the class factory we release the instance.
301231
m_tracerInstance.release();
302-
303-
if (GHR != S_OK)
304-
{
305-
GHR = result;
306-
}
307-
}
308-
else
309-
{
310-
GHR = S_OK;
232+
GHR = result;
311233
}
312234
}
313235

314236
if (m_customInstance != nullptr)
315237
{
316-
HRESULT result = m_customInstance->LoadInstance();
238+
HRESULT result = m_customInstance->LoadInstance(pUnkOuter, riid);
317239
if (FAILED(result))
318240
{
319-
Log::Warn("DynamicDispatcherImpl::LoadInstance: Error trying to load the custom instance in: ",
320-
m_customInstance->GetFilePath(), ", error code: ", result);
241+
Log::Warn("DynamicDispatcherImpl::LoadInstance: Error trying to load the custom instance in: ", m_customInstance->GetFilePath());
321242

322243
// If we cannot load the class factory we release the instance.
323244
m_customInstance.release();
324-
325-
if (GHR != S_OK)
326-
{
327-
GHR = result;
328-
}
329-
}
330-
else
331-
{
332-
GHR = S_OK;
245+
GHR = result;
333246
}
334247
}
335248

@@ -404,4 +317,4 @@ namespace datadog::shared::nativeloader
404317
return m_customInstance.get();
405318
}
406319

407-
} // namespace datadog::shared::nativeloader
320+
} // namespace datadog::shared::nativeloader

shared/src/Datadog.Trace.ClrProfiler.Native/dynamic_dispatcher.h

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,11 @@ namespace datadog::shared::nativeloader
1818
//
1919
class IDynamicDispatcher
2020
{
21-
protected:
22-
virtual void LoadConfiguration(fs::path&& configFilePath) = 0;
23-
2421
public:
2522
virtual ~IDynamicDispatcher() = default;
26-
virtual HRESULT Initialize() = 0;
23+
virtual void LoadConfiguration(fs::path&& configFilePath) = 0;
24+
virtual HRESULT LoadClassFactory(REFIID riid) = 0;
25+
virtual HRESULT LoadInstance(IUnknown* pUnkOuter, REFIID riid) = 0;
2726
virtual HRESULT STDMETHODCALLTYPE DllCanUnloadNow() = 0;
2827
virtual IDynamicInstance* GetContinuousProfilerInstance() = 0;
2928
virtual IDynamicInstance* GetTracerInstance() = 0;
@@ -39,22 +38,16 @@ namespace datadog::shared::nativeloader
3938
std::unique_ptr<IDynamicInstance> m_continuousProfilerInstance;
4039
std::unique_ptr<IDynamicInstance> m_tracerInstance;
4140
std::unique_ptr<IDynamicInstance> m_customInstance;
42-
void LoadConfiguration(fs::path&& configFilePath) override;
4341

4442
public:
4543
DynamicDispatcherImpl();
46-
HRESULT Initialize() override;
44+
void LoadConfiguration(fs::path&& configFilePath) override;
45+
HRESULT LoadClassFactory(REFIID riid) override;
46+
HRESULT LoadInstance(IUnknown* pUnkOuter, REFIID riid) override;
4747
HRESULT STDMETHODCALLTYPE DllCanUnloadNow() override;
4848
IDynamicInstance* GetContinuousProfilerInstance() override;
4949
IDynamicInstance* GetTracerInstance() override;
5050
IDynamicInstance* GetCustomInstance() override;
51-
52-
private:
53-
HRESULT LoadClassFactory(REFIID riid);
54-
HRESULT LoadInstance();
55-
56-
bool m_initialized;
57-
HRESULT m_initializationResult;
5851
};
5952

6053
} // namespace datadog::shared::nativeloader

shared/src/Datadog.Trace.ClrProfiler.Native/dynamic_instance.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ namespace datadog::shared::nativeloader
6565
return res;
6666
}
6767

68-
HRESULT DynamicInstanceImpl::LoadInstance()
68+
HRESULT DynamicInstanceImpl::LoadInstance(IUnknown* pUnkOuter, REFIID riid)
6969
{
7070
Log::Debug("DynamicInstanceImpl::LoadInstance");
7171

shared/src/Datadog.Trace.ClrProfiler.Native/dynamic_instance.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace datadog::shared::nativeloader
2020
public:
2121
virtual ~IDynamicInstance() = default;
2222
virtual HRESULT LoadClassFactory(REFIID riid) = 0;
23-
virtual HRESULT LoadInstance() = 0;
23+
virtual HRESULT LoadInstance(IUnknown* pUnkOuter, REFIID riid) = 0;
2424
virtual HRESULT STDMETHODCALLTYPE DllCanUnloadNow() = 0;
2525
virtual ICorProfilerCallback10* GetProfilerCallback() = 0;
2626
virtual std::string GetFilePath() = 0;
@@ -42,7 +42,7 @@ namespace datadog::shared::nativeloader
4242
DynamicInstanceImpl(const std::string& filePath, const std::string& clsid);
4343
~DynamicInstanceImpl() override;
4444
HRESULT LoadClassFactory(REFIID riid) override;
45-
HRESULT LoadInstance() override;
45+
HRESULT LoadInstance(IUnknown* pUnkOuter, REFIID riid) override;
4646
HRESULT STDMETHODCALLTYPE DllCanUnloadNow() override;
4747
ICorProfilerCallback10* GetProfilerCallback() override;
4848
std::string GetFilePath() override;

0 commit comments

Comments
 (0)