Skip to content

Commit b3c9cd1

Browse files
chynesNRchynesNR
authored
fix: Revert recent Profiler warning fixes to address reported instability (#2663)
1 parent 1a8a6ec commit b3c9cd1

23 files changed

+82
-115
lines changed

‎src/Agent/NewRelic/Home/Home.csproj

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
</Target>
1414

1515
<ItemGroup>
16-
<PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.27.0.15"/>
16+
<PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.27.0.20"/>
1717
</ItemGroup>
1818

1919
</Project>

‎src/Agent/NewRelic/Profiler/Common/CorStandIn.h

+1-3
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55
#pragma warning(push)
6-
// Since this isn't our code, we don't want to mess with it. These warnings can be safely ignored.
7-
#pragma warning(disable: 4458) // Scope hides class member with same name.
8-
#pragma warning(disable: 26495) // Uninitialized member variable, even if it's always set before use.
6+
#pragma warning(disable : 4458)
97
#include <corhlpr.h>
108
#pragma warning(pop)

‎src/Agent/NewRelic/Profiler/Configuration/Configuration.h

+16-16
Original file line numberDiff line numberDiff line change
@@ -47,24 +47,24 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
4747
, _ignoreList(new IgnoreInstrumentationList())
4848
{
4949
try {
50-
auto globalNewRelicConfigurationDocument = std::make_shared<rapidxml::xml_document<xchar_t>>();
51-
globalNewRelicConfigurationDocument->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(globalNewRelicConfiguration.c_str()));
50+
rapidxml::xml_document<xchar_t> globalNewRelicConfigurationDocument;
51+
globalNewRelicConfigurationDocument.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(globalNewRelicConfiguration.c_str()));
5252

53-
auto globalNewRelicConfigurationNode = GetConfigurationNode(globalNewRelicConfigurationDocument);
53+
auto globalNewRelicConfigurationNode = GetConfigurationNode(globalNewRelicConfigurationDocument);
5454
if (globalNewRelicConfigurationNode == nullptr)
5555
{
5656
LogError(L"Unable to locate configuration node in the global newrelic.config file.");
5757
throw ConfigurationException();
5858
}
5959

6060
auto appliedNewRelicConfigurationNode = globalNewRelicConfigurationNode;
61-
auto localNewRelicConfigurationDocument = std::make_shared<rapidxml::xml_document<xchar_t>>();
6261

6362
if (localNewRelicConfiguration.second)
6463
{
6564
try
6665
{
67-
localNewRelicConfigurationDocument->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(localNewRelicConfiguration.first.c_str()));
66+
rapidxml::xml_document<xchar_t> localNewRelicConfigurationDocument;
67+
localNewRelicConfigurationDocument.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(localNewRelicConfiguration.first.c_str()));
6868

6969
auto localNewRelicConfigurationNode = GetConfigurationNode(localNewRelicConfigurationDocument);
7070
if (localNewRelicConfigurationNode == nullptr)
@@ -92,7 +92,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
9292
SetLogLevel(appliedNewRelicConfigurationNode);
9393
SetInstrumentationData(appliedNewRelicConfigurationNode);
9494
SetApplicationPools(appliedNewRelicConfigurationNode);
95-
95+
9696
} catch (const rapidxml::parse_error& exception) {
9797
// We log two separate error messages here because sometimes the logging macros hang when
9898
// logging the "where" contents
@@ -196,15 +196,15 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
196196
return _logLevel;
197197
}
198198

199-
bool GetConsoleLogging() const
199+
bool GetConsoleLogging()
200200
{
201201
return _consoleLogging;
202202
}
203-
bool GetLoggingEnabled() const
203+
bool GetLoggingEnabled()
204204
{
205205
return _loggingEnabled;
206206
}
207-
IgnoreInstrumentationListPtr GetIgnoreInstrumentationList() const
207+
IgnoreInstrumentationListPtr GetIgnoreInstrumentationList()
208208
{
209209
return _ignoreList;
210210
}
@@ -224,9 +224,9 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
224224
std::shared_ptr<NewRelic::Profiler::Logger::IFileDestinationSystemCalls> _systemCalls;
225225
IgnoreInstrumentationListPtr _ignoreList;
226226

227-
rapidxml::xml_node<xchar_t>* GetConfigurationNode(const std::shared_ptr<rapidxml::xml_document<xchar_t>> document)
227+
rapidxml::xml_node<xchar_t>* GetConfigurationNode(const rapidxml::xml_document<xchar_t>& document)
228228
{
229-
auto configurationNode = document->first_node(_X("configuration"), 0, false);
229+
auto configurationNode = document.first_node(_X("configuration"), 0, false);
230230
if (configurationNode == nullptr) {
231231
return nullptr;
232232
}
@@ -294,7 +294,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
294294
_logLevel = TryParseLogLevel(level);
295295
}
296296

297-
Logger::Level TryParseLogLevel(const xstring_t& logText) const
297+
Logger::Level TryParseLogLevel(const xstring_t& logText)
298298
{
299299
if (Strings::AreEqualCaseInsensitive(logText, _X("off"))) {
300300
return Logger::Level::LEVEL_ERROR;
@@ -423,8 +423,8 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
423423
if (applicationConfiguration.empty())
424424
return;
425425

426-
auto document = std::make_shared<rapidxml::xml_document<xchar_t>>();
427-
document->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(applicationConfiguration.c_str()));
426+
rapidxml::xml_document<xchar_t> document;
427+
document.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(applicationConfiguration.c_str()));
428428
auto configurationNode = GetConfigurationNode(document);
429429

430430
auto appSettingsNode = configurationNode->first_node(_X("appSettings"), 0, false);
@@ -468,7 +468,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
468468
static bool IsProcessInProcessList(const ProcessesPtr& processes, const xstring_t& processName)
469469
{
470470
// check the processes loaded from configuration
471-
for (auto& validProcessName : *processes) {
471+
for (auto validProcessName : *processes) {
472472
if (Strings::EndsWith(processName, validProcessName)) {
473473
return true;
474474
}
@@ -498,7 +498,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
498498
return isIis;
499499
}
500500

501-
bool ShouldInstrumentApplicationPool(const xstring_t& appPoolId) const
501+
bool ShouldInstrumentApplicationPool(const xstring_t& appPoolId)
502502
{
503503
if (ApplicationPoolIsOnBlackList(appPoolId, _applicationPoolsBlackList)) {
504504
LogInfo(_X("This application pool (") + appPoolId + _X(") is explicitly configured to NOT be instrumented."));

‎src/Agent/NewRelic/Profiler/Configuration/IgnoreInstrumentation.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
5050
}
5151

5252
private:
53-
bool Matches(xstring_t assembly, xstring_t className) const
53+
bool Matches(xstring_t assembly, xstring_t className)
5454
{
5555
if (!Strings::AreEqualCaseInsensitive(AssemblyName, assembly))
5656
{

‎src/Agent/NewRelic/Profiler/Configuration/InstrumentationConfiguration.h

+8-8
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
3333
, _foundServerlessInstrumentationPoint(false)
3434
{
3535
// pull instrumentation points from every xml string
36-
for (auto& instrumentationXml : *instrumentationXmls)
36+
for (auto instrumentationXml : *instrumentationXmls)
3737
{
3838
try
3939
{
@@ -64,13 +64,13 @@ namespace NewRelic { namespace Profiler { namespace Configuration
6464
, _systemCalls(nullptr)
6565
, _foundServerlessInstrumentationPoint(false)
6666
{
67-
for (auto& instrumentationPoint : *instrumentationPoints)
67+
for (auto instrumentationPoint : *instrumentationPoints)
6868
{
6969
AddInstrumentationPointToCollectionsIfNotIgnored(instrumentationPoint);
7070
}
7171
}
7272

73-
uint16_t GetInvalidFileCount() const
73+
uint16_t GetInvalidFileCount()
7474
{
7575
return _invalidFileCount;
7676
}
@@ -100,7 +100,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
100100
// We may have multiple matching instrumentation points that target different assembly versions. See if we can find one that meets
101101
// the version requirements
102102
AssemblyVersion foundVersion(function->GetAssemblyProps());
103-
for (auto& instPoint : instPoints)
103+
for (auto instPoint : instPoints)
104104
{
105105
if ((instPoint->MinVersion != nullptr) && (foundVersion < *instPoint->MinVersion))
106106
{
@@ -236,9 +236,9 @@ namespace NewRelic { namespace Profiler { namespace Configuration
236236

237237
void GetInstrumentationPoints(xstring_t instrumentationXml)
238238
{
239-
auto document = std::make_shared<rapidxml::xml_document<xchar_t>>();
240-
document->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(instrumentationXml.c_str()));
241-
auto extensionNode = document->first_node(_X("extension"), 0, false);
239+
rapidxml::xml_document<xchar_t> document;
240+
document.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(instrumentationXml.c_str()));
241+
auto extensionNode = document.first_node(_X("extension"), 0, false);
242242
if (extensionNode == nullptr)
243243
{
244244
LogWarn(L"extension node not found in instrumentation file. Please validate your instrumentation files against extensions/extension.xsd or contact New Relic support.");
@@ -405,7 +405,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
405405
// if the ClassName includes multiple classes, we have to split this into multiple instrumentation points
406406
auto instrumentationPoints = SplitInstrumentationPointsOnClassNames(instrumentationPoint);
407407

408-
for (auto& iPoint : instrumentationPoints) {
408+
for (auto iPoint : instrumentationPoints) {
409409

410410
// finally add the new instrumentation point(s) to our set of instrumentation points
411411
// Note that there may be "duplicated" instrumentation points that target different assembly versions

‎src/Agent/NewRelic/Profiler/Configuration/InstrumentationPoint.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
5252
ParametersMatch(other);
5353
}
5454

55-
xstring_t ToString() const
55+
xstring_t ToString()
5656
{
5757
if (Parameters == nullptr)
5858
{
@@ -63,7 +63,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
6363
}
6464
}
6565

66-
xstring_t GetMatchKey() const
66+
xstring_t GetMatchKey()
6767
{
6868
return Parameters == nullptr
6969
? GetMatchKey(AssemblyName, ClassName, MethodName)
@@ -82,7 +82,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
8282

8383

8484
private:
85-
bool ParametersMatch(const InstrumentationPoint& other) const
85+
bool ParametersMatch(const InstrumentationPoint& other)
8686
{
8787
// nullptr means no parameters attribute was supplied in configuration, suggesting that we should instrument all overloads
8888
if (this->Parameters == nullptr)

‎src/Agent/NewRelic/Profiler/MethodRewriter/ExceptionHandlerManipulator.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -262,12 +262,12 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
262262
// shift the original clauses up to the correct
263263
for (uint32_t i = 0; i < _originalExceptionClauseCount; ++i)
264264
{
265-
auto& clause = _exceptionClauses[i];
265+
auto clause = _exceptionClauses[i];
266266
clause->ShiftOffsets(userCodeOffset);
267267
}
268268

269269
// append the clauses
270-
for (auto& clause : _exceptionClauses)
270+
for (auto clause : _exceptionClauses)
271271
{
272272
auto clauseBytes = clause->GetBytes();
273273
bytes->insert(bytes->end(), clauseBytes->begin(), clauseBytes->end());
@@ -276,7 +276,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
276276
return bytes;
277277
}
278278

279-
uint32_t GetOriginalExceptionClauseCount() const
279+
uint32_t GetOriginalExceptionClauseCount()
280280
{
281281
return _originalExceptionClauseCount;
282282
}

‎src/Agent/NewRelic/Profiler/MethodRewriter/FunctionManipulator.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
350350
_instructions->Append(CEE_NEWARR, _X("[mscorlib]System.Object"));
351351
uint32_t index = 0;
352352

353-
for (auto& func : elementLoadLambdas)
353+
for (auto func : elementLoadLambdas)
354354
{
355355
auto nextIndex = index++;
356356
// get an extra copy of the array (it will be popped off the stack each time we add an element to it)

‎src/Agent/NewRelic/Profiler/MethodRewriter/InstructionSet.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
403403

404404
void AppendTryEnd()
405405
{
406-
auto& exception = _exceptionStack.top();
406+
auto exception = _exceptionStack.top();
407407
if (exception->_tryLength != 0)
408408
{
409409
LogError(L"Attempted to set try close on the same exception twice.");
@@ -414,7 +414,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
414414

415415
void AppendCatchStart(uint32_t typeToken)
416416
{
417-
auto& exception = _exceptionStack.top();
417+
auto exception = _exceptionStack.top();
418418
if (exception->_handlerOffset != 0)
419419
{
420420
LogError(L"Attempted to set catch start on the same exception twice.");
@@ -439,7 +439,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
439439

440440
void AppendCatchEnd()
441441
{
442-
auto& exception = _exceptionStack.top();
442+
auto exception = _exceptionStack.top();
443443
if (exception->_handlerLength != 0)
444444
{
445445
LogError(L"Attempted to set catch end on the same exception twice.");

‎src/Agent/NewRelic/Profiler/MethodRewriter/InstrumentFunctionManipulator.h

+2-5
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
1616
public:
1717
InstrumentFunctionManipulator(IFunctionPtr function, InstrumentationSettingsPtr instrumentationSettings) :
1818
FunctionManipulator(function),
19-
_instrumentationSettings(instrumentationSettings),
20-
_userExceptionLocalIndex(0),
21-
_resultLocalIndex(0),
22-
_tracerLocalIndex(0)
19+
_instrumentationSettings(instrumentationSettings)
2320
{
2421
if (_function->Preprocess()) {
2522
Initialize();
@@ -231,4 +228,4 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
231228
_resultLocalIndex = AppendReturnTypeLocal(_newLocalVariablesSignature, _methodSignature);
232229
}
233230
};
234-
}}}
231+
}}}

‎src/Agent/NewRelic/Profiler/MethodRewriter/MethodRewriter.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter {
5454

5555
auto instrumentationPoints = _instrumentationConfiguration->GetInstrumentationPoints();
5656

57-
for (auto& instrumentationPoint : *instrumentationPoints) {
57+
for (auto instrumentationPoint : *instrumentationPoints) {
5858

5959
_instrumentedAssemblies->emplace(instrumentationPoint->AssemblyName);
6060
_instrumentedFunctionNames->emplace(instrumentationPoint->MethodName);
@@ -74,7 +74,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter {
7474
std::set<Configuration::InstrumentationPointPtr> GetAssemblyInstrumentation(xstring_t assemblyName)
7575
{
7676
std::set<Configuration::InstrumentationPointPtr> set;
77-
for (auto& instrumentationPoint : *_instrumentationConfiguration->GetInstrumentationPoints().get()) {
77+
for (auto instrumentationPoint : *_instrumentationConfiguration->GetInstrumentationPoints().get()) {
7878
if (assemblyName == instrumentationPoint->AssemblyName) {
7979
set.emplace(instrumentationPoint);
8080
}

‎src/Agent/NewRelic/Profiler/ModuleInjector/ModuleInjector.h

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
#include "../Logging/Logger.h"
1111
#include "../Sicily/Sicily.h"
1212
#include "IModule.h"
13-
#include "../Profiler/Exceptions.h"
1413

1514
namespace NewRelic { namespace Profiler { namespace ModuleInjector
1615
{

0 commit comments

Comments
 (0)