From 857797d9ba9be6f82b83ae6c29b51d490d41cf87 Mon Sep 17 00:00:00 2001 From: Noah Falk Date: Mon, 28 Oct 2019 17:59:52 -0700 Subject: [PATCH] Support large EventSource filter args (#27500) 1. Fix NullReferenceException. When the filter args exceeded the hard-coded size limit GetDataFromController would return data = null. The code previously asserted that data was not null and triggered NRE when it was. The fix correctly null checks the value instead of asserting and uses an empty args dictionary in this case, the same as if filter args had been empty to begin with. 2. ETW has always limited filter args to 1024 bytes but EventPipe has no such restriction. When using DiagnosticSourceEventSource it can be useful to specify a larger filter arg blob. I can't do anything about ETW's restriction but there is no need for the runtime to force EventPipe to be equally limited. The larger size also reduces the chance that we need to hit the fallback path above causing filter args to be ignored. --- .../Diagnostics/Tracing/EventProvider.cs | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventProvider.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventProvider.cs index 232a82c0d592..b1787019a163 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventProvider.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventProvider.cs @@ -294,19 +294,22 @@ private unsafe void EtwEnableCallBack( GetDataFromController(etwSessionId, filterData, out command, out data, out keyIndex)) { args = new Dictionary(4); - Debug.Assert(data != null); - while (keyIndex < data.Length) + // data can be null if the filterArgs had a very large size which failed our sanity check + if (data != null) { - int keyEnd = FindNull(data, keyIndex); - int valueIdx = keyEnd + 1; - int valueEnd = FindNull(data, valueIdx); - if (valueEnd < data.Length) + while (keyIndex < data.Length) { - string key = System.Text.Encoding.UTF8.GetString(data, keyIndex, keyEnd - keyIndex); - string value = System.Text.Encoding.UTF8.GetString(data, valueIdx, valueEnd - valueIdx); - args[key] = value; + int keyEnd = FindNull(data, keyIndex); + int valueIdx = keyEnd + 1; + int valueEnd = FindNull(data, valueIdx); + if (valueEnd < data.Length) + { + string key = System.Text.Encoding.UTF8.GetString(data, keyIndex, keyEnd - keyIndex); + string value = System.Text.Encoding.UTF8.GetString(data, valueIdx, valueEnd - valueIdx); + args[key] = value; + } + keyIndex = valueEnd + 1; } - keyIndex = valueEnd + 1; } } @@ -619,7 +622,10 @@ private unsafe bool GetDataFromController(int etwSessionId, } else { - if (filterData->Ptr != 0 && 0 < filterData->Size && filterData->Size <= 1024) + // ETW limited filter data to 1024 bytes but EventPipe doesn't. DiagnosticSourceEventSource + // can legitimately use large filter data buffers to encode a large set of events and properties + // that should be gathered so I am bumping the limit from 1K -> 100K. + if (filterData->Ptr != 0 && 0 < filterData->Size && filterData->Size <= 100*1024) { data = new byte[filterData->Size]; Marshal.Copy((IntPtr)filterData->Ptr, data, 0, data.Length);