Skip to content

Conversation

@nickyMcDonald
Copy link
Contributor

Here is a fix for issue #78006 that works with win11 .NET 7.

The EventPipeEventDispatcher cycles through EventPipeInternal via QCall and sends them to be processed by the NativeRuntimeEventSource.

while (!m_stopDispatchTask && EventPipeInternal.GetNextEvent(m_sessionID, &instanceData))
{
eventsReceived = true;
// Filter based on provider.
if (instanceData.ProviderID == m_RuntimeProviderID)
{
// Dispatch the event.
ReadOnlySpan<byte> payload = new ReadOnlySpan<byte>((void*)instanceData.Payload, (int)instanceData.PayloadLength);
DateTime dateTimeStamp = TimeStampToDateTime(instanceData.TimeStamp);
NativeRuntimeEventSource.Log.ProcessEvent(instanceData.EventID, instanceData.ThreadID, dateTimeStamp, instanceData.ActivityId, instanceData.ChildActivityId, payload);
}
}

From there they are sent to be decoded by the EventPipePayloadDecoder.
object[] decodedPayloadFields = EventPipePayloadDecoder.DecodePayload(ref m_eventData[eventID], payload);

There is where the NRE occurs because the metadata is uninitialized. Parameters is default null and attempting to get its length results in NRE. Looking back at the NativeRuntimeEventSource the NRE occurs with eventID 64 (ThreadPoolIODequeue).
internal static object[] DecodePayload(ref EventSource.EventMetadata metadata, ReadOnlySpan<byte> payload)
{
ParameterInfo[] parameters = metadata.Parameters;
object[] decodedFields = new object[parameters.Length];

In the EventSource. There are no BindingFlags for static methods. This is important because any static methods from the NativeRuntimeEventSource won’t get loaded. Because this data is used to initialize m_eventData values, any methods left out will leave uninitialized slots in m_eventData. Slot 64 being one of them.
MethodInfo[] methods = eventSourceType.GetMethods(BindingFlags.DeclaredOnly | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance);

The fix will allow for event 64 and 63 to be loaded into m_eventData before pr #66333 again.

remove static modifiers from lines:
 - 156
 - 191
in:
src\libraries\System.Private.CoreLib\src\System\Threading\NativeRuntimeEventSource.PortableThreadPool.NativeSinks.cs
@ghost ghost added area-System.Threading community-contribution Indicates that the PR has been added by a community member labels Apr 30, 2023
@ghost
Copy link

ghost commented Apr 30, 2023

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

Here is a fix for issue #78006 that works with win11 .NET 7.

The EventPipeEventDispatcher cycles through EventPipeInternal via QCall and sends them to be processed by the NativeRuntimeEventSource.

while (!m_stopDispatchTask && EventPipeInternal.GetNextEvent(m_sessionID, &instanceData))
{
eventsReceived = true;
// Filter based on provider.
if (instanceData.ProviderID == m_RuntimeProviderID)
{
// Dispatch the event.
ReadOnlySpan<byte> payload = new ReadOnlySpan<byte>((void*)instanceData.Payload, (int)instanceData.PayloadLength);
DateTime dateTimeStamp = TimeStampToDateTime(instanceData.TimeStamp);
NativeRuntimeEventSource.Log.ProcessEvent(instanceData.EventID, instanceData.ThreadID, dateTimeStamp, instanceData.ActivityId, instanceData.ChildActivityId, payload);
}
}

From there they are sent to be decoded by the EventPipePayloadDecoder.
object[] decodedPayloadFields = EventPipePayloadDecoder.DecodePayload(ref m_eventData[eventID], payload);

There is where the NRE occurs because the metadata is uninitialized. Parameters is default null and attempting to get its length results in NRE. Looking back at the NativeRuntimeEventSource the NRE occurs with eventID 64 (ThreadPoolIODequeue).
internal static object[] DecodePayload(ref EventSource.EventMetadata metadata, ReadOnlySpan<byte> payload)
{
ParameterInfo[] parameters = metadata.Parameters;
object[] decodedFields = new object[parameters.Length];

In the EventSource. There are no BindingFlags for static methods. This is important because any static methods from the NativeRuntimeEventSource won’t get loaded. Because this data is used to initialize m_eventData values, any methods left out will leave uninitialized slots in m_eventData. Slot 64 being one of them.
MethodInfo[] methods = eventSourceType.GetMethods(BindingFlags.DeclaredOnly | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance);

The fix will allow for event 64 and 63 to be loaded into m_eventData before pr #66333 again.

Author: n77y
Assignees: -
Labels:

area-System.Threading

Milestone: -

@jkotas
Copy link
Member

jkotas commented Apr 30, 2023

@jkotas
Copy link
Member

jkotas commented Apr 30, 2023

@hoyosjs
Copy link
Member

hoyosjs commented May 9, 2023

Hmm... tracing/runtimeeventsource/nativeruntimeeventsource/nativeruntimeeventsource.sh is now timing out and a dump was collected on macOS x64

@nickyMcDonald
Copy link
Contributor Author

Hmm... tracing/runtimeeventsource/nativeruntimeeventsource/nativeruntimeeventsource.sh is now timing out and a dump was collected on macOS x64

After adding tests to NativeRuntimeEventSourceTest.cs and returning the static modifiers to NativeRuntimeEventSource.PortableThreadPool.NativeSinks.cs the tests successfully failed. I will now remove the static modifiers and the tracing tests should now pass.

@tommcdon
Copy link
Member

@davmason ptal

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Threading community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants