Skip to content

Commit

Permalink
Support large EventSource filter args (dotnet#27522)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
noahfalk authored and danmoseley committed Oct 31, 2019
1 parent 934ef61 commit a0a1e56
Showing 1 changed file with 17 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -322,19 +322,22 @@ unsafe void EtwEnableCallBack(
GetDataFromController(etwSessionId, filterData, out command, out data, out keyIndex))
{
args = new Dictionary<string, string?>(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;
}
}

Expand Down Expand Up @@ -635,7 +638,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);
Expand Down

0 comments on commit a0a1e56

Please sign in to comment.