Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce 64KB event payload size limit on EventPipe #50600

Merged
merged 7 commits into from
Apr 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions src/native/eventpipe/ep-buffer-manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,7 @@ ep_buffer_manager_alloc (

instance->session = session;
instance->size_of_all_buffers = 0;
instance->num_oversized_events_dropped = 0;

#ifdef EP_CHECKED_BUILD
instance->num_buffers_allocated = 0;
Expand Down Expand Up @@ -887,6 +888,8 @@ ep_buffer_manager_write_event (
bool alloc_new_buffer = false;
EventPipeBuffer *buffer = NULL;
EventPipeThreadSessionState *session_state = NULL;
EventPipeStackContents stack_contents;
EventPipeStackContents *current_stack_contents = NULL;

EP_ASSERT (buffer_manager != NULL);
EP_ASSERT (ep_event != NULL);
Expand All @@ -897,12 +900,23 @@ ep_buffer_manager_write_event (
// Before we pick a buffer, make sure the event is enabled.
ep_return_false_if_nok (ep_event_is_enabled (ep_event));

// Check that the payload size is less than 64 KB (max size for ETW events)
if (ep_event_payload_get_size (payload) > 64 * 1024)
{
ep_rt_atomic_inc_int64_t (&buffer_manager->num_oversized_events_dropped);
sywhang marked this conversation as resolved.
Show resolved Hide resolved
EventPipeThread *current_thread = ep_thread_get();
ep_rt_spin_lock_handle_t *thread_lock = ep_thread_get_rt_lock_ref (current_thread);
EP_SPIN_LOCK_ENTER (thread_lock, section1)
session_state = ep_thread_get_or_create_session_state (current_thread, session);
ep_thread_session_state_increment_sequence_number (session_state);
EP_SPIN_LOCK_EXIT (thread_lock, section1)
return false;
}

// Check to see an event thread was specified. If not, then use the current thread.
if (event_thread == NULL)
event_thread = thread;

EventPipeStackContents stack_contents;
EventPipeStackContents *current_stack_contents;
current_stack_contents = ep_stack_contents_init (&stack_contents);
if (stack == NULL && ep_event_get_need_stack (ep_event) && !ep_session_get_rundown_enabled (session)) {
ep_walk_managed_stack_for_current_thread (current_stack_contents);
Expand All @@ -917,9 +931,9 @@ ep_buffer_manager_write_event (
ep_rt_spin_lock_handle_t *thread_lock;
thread_lock = ep_thread_get_rt_lock_ref (current_thread);

EP_SPIN_LOCK_ENTER (thread_lock, section1)
EP_SPIN_LOCK_ENTER (thread_lock, section2)
session_state = ep_thread_get_or_create_session_state (current_thread, session);
ep_raise_error_if_nok_holding_spin_lock (session_state != NULL, section1);
ep_raise_error_if_nok_holding_spin_lock (session_state != NULL, section2);

buffer = ep_thread_session_state_get_write_buffer (session_state);
if (!buffer) {
Expand All @@ -931,7 +945,7 @@ ep_buffer_manager_write_event (
else
alloc_new_buffer = true;
}
EP_SPIN_LOCK_EXIT (thread_lock, section1)
EP_SPIN_LOCK_EXIT (thread_lock, section2)

// alloc_new_buffer is reused below to detect if overflow happened, so cache it here to see if we should
// signal the reader thread
Expand All @@ -951,23 +965,23 @@ ep_buffer_manager_write_event (
// This lock looks unnecessary for the sequence number, but didn't want to
// do a broader refactoring to take it out. If it shows up as a perf
// problem then we should.
EP_SPIN_LOCK_ENTER (thread_lock, section2)
EP_SPIN_LOCK_ENTER (thread_lock, section3)
ep_thread_session_state_increment_sequence_number (session_state);
EP_SPIN_LOCK_EXIT (thread_lock, section2)
EP_SPIN_LOCK_EXIT (thread_lock, section3)
} else {
current_thread = ep_thread_get ();
EP_ASSERT (current_thread != NULL);

thread_lock = ep_thread_get_rt_lock_ref (current_thread);
EP_SPIN_LOCK_ENTER (thread_lock, section3)
EP_SPIN_LOCK_ENTER (thread_lock, section4)
ep_thread_session_state_set_write_buffer (session_state, buffer);
// Try to write the event after we allocated a buffer.
// This is the first time if the thread had no buffers before the call to this function.
// This is the second time if this thread did have one or more buffers, but they were full.
alloc_new_buffer = !ep_buffer_write_event (buffer, event_thread, session, ep_event, payload, activity_id, related_activity_id, stack);
EP_ASSERT(!alloc_new_buffer);
ep_thread_session_state_increment_sequence_number (session_state);
EP_SPIN_LOCK_EXIT (thread_lock, section3)
EP_SPIN_LOCK_EXIT (thread_lock, section4)
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/native/eventpipe/ep-buffer-manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ struct _EventPipeBufferManager_Internal {
// The total amount of allocations we can do after one sequence
// point before triggering the next one
size_t sequence_point_alloc_budget;
// number of times an event was dropped due to it being too
// large to fit in the 64KB size limit
volatile int64_t num_oversized_events_dropped;

#ifdef EP_CHECKED_BUILD
volatile int64_t num_events_stored;
Expand Down
89 changes: 89 additions & 0 deletions src/tests/tracing/eventpipe/bigevent/bigevent.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics.Tracing;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
using Microsoft.Diagnostics.Tools.RuntimeClient;
using Microsoft.Diagnostics.Tracing;
using Tracing.Tests.Common;
using Microsoft.Diagnostics.Tracing.Parsers.Clr;

namespace Tracing.Tests.BigEventsValidation
{

public sealed class BigEventSource : EventSource
{
private static string bigString = new String('a', 100 * 1024);
sywhang marked this conversation as resolved.
Show resolved Hide resolved
private static string smallString = new String('a', 10);

private BigEventSource()
{
}

public static BigEventSource Log = new BigEventSource();

public void BigEvent()
{
WriteEvent(1, bigString);
}

public void SmallEvent()
{
WriteEvent(2, smallString);
}
}


public class BigEventsValidation
{
public static int Main(string[] args)
{
// This test tries to send a big event (>100KB) and checks that the app does not crash
// See https://github.com/dotnet/runtime/issues/50515 for the regression issue
var providers = new List<Provider>()
{
new Provider("BigEventSource")
};

var configuration = new SessionConfiguration(circularBufferSizeMB: 1024, format: EventPipeSerializationFormat.NetTrace, providers: providers);
return IpcTraceTest.RunAndValidateEventCounts(_expectedEventCounts, _eventGeneratingAction, configuration, _Verify);
}

private static Dictionary<string, ExpectedEventCount> _expectedEventCounts = new Dictionary<string, ExpectedEventCount>()
{
{ "BigEventSource", -1 }
};

private static Action _eventGeneratingAction = () =>
{
// Write 10 big events
for (int i = 0; i < 10; i++)
{
BigEventSource.Log.BigEvent();
}
// Write 10 small events
for (int i = 0; i < 10; i++)
{
BigEventSource.Log.SmallEvent();
}
};

private static Func<EventPipeEventSource, Func<int>> _Verify = (source) =>
{
bool hasSmallEvent = false;
source.Dynamic.All += (TraceEvent data) =>
{
if (data.EventName == "SmallEvent")
{
hasSmallEvent = true;
}
};
return () => hasSmallEvent ? 100 : -1;
};
}
}
15 changes: 15 additions & 0 deletions src/tests/tracing/eventpipe/bigevent/bigevent.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworkIdentifier>.NETCoreApp</TargetFrameworkIdentifier>
<OutputType>exe</OutputType>
<CLRTestKind>BuildAndRun</CLRTestKind>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestPriority>0</CLRTestPriority>
<JitOptimizationSensitive>true</JitOptimizationSensitive>
<UnloadabilityIncompatible>true</UnloadabilityIncompatible>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
<ProjectReference Include="../common/common.csproj" />
</ItemGroup>
</Project>