From bdd7ca0d78ce86c2dc0ab24df42a09ef514a771c Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Mon, 5 Apr 2021 16:27:39 -0700 Subject: [PATCH] Enforce 64KB event payload size limit on EventPipe (#50600) * allow >100KB events to be written * prevent events > 64KB from being written * code review feedback * increment seq num * fix build * fix build on clang * fix mono build --- src/native/eventpipe/ep-buffer-manager.c | 32 +++++-- src/native/eventpipe/ep-buffer-manager.h | 3 + .../tracing/eventpipe/bigevent/bigevent.cs | 89 +++++++++++++++++++ .../eventpipe/bigevent/bigevent.csproj | 15 ++++ 4 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 src/tests/tracing/eventpipe/bigevent/bigevent.cs create mode 100644 src/tests/tracing/eventpipe/bigevent/bigevent.csproj diff --git a/src/native/eventpipe/ep-buffer-manager.c b/src/native/eventpipe/ep-buffer-manager.c index 74d8b542e726b..2355d1361ae7e 100644 --- a/src/native/eventpipe/ep-buffer-manager.c +++ b/src/native/eventpipe/ep-buffer-manager.c @@ -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; @@ -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); @@ -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); + 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); @@ -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) { @@ -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 @@ -951,15 +965,15 @@ 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. @@ -967,7 +981,7 @@ ep_buffer_manager_write_event ( 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) } } diff --git a/src/native/eventpipe/ep-buffer-manager.h b/src/native/eventpipe/ep-buffer-manager.h index e9fe64d25e63f..822b8450279f9 100644 --- a/src/native/eventpipe/ep-buffer-manager.h +++ b/src/native/eventpipe/ep-buffer-manager.h @@ -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; diff --git a/src/tests/tracing/eventpipe/bigevent/bigevent.cs b/src/tests/tracing/eventpipe/bigevent/bigevent.cs new file mode 100644 index 0000000000000..dd5294188efb1 --- /dev/null +++ b/src/tests/tracing/eventpipe/bigevent/bigevent.cs @@ -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); + 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() + { + new Provider("BigEventSource") + }; + + var configuration = new SessionConfiguration(circularBufferSizeMB: 1024, format: EventPipeSerializationFormat.NetTrace, providers: providers); + return IpcTraceTest.RunAndValidateEventCounts(_expectedEventCounts, _eventGeneratingAction, configuration, _Verify); + } + + private static Dictionary _expectedEventCounts = new Dictionary() + { + { "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> _Verify = (source) => + { + bool hasSmallEvent = false; + source.Dynamic.All += (TraceEvent data) => + { + if (data.EventName == "SmallEvent") + { + hasSmallEvent = true; + } + }; + return () => hasSmallEvent ? 100 : -1; + }; + } +} diff --git a/src/tests/tracing/eventpipe/bigevent/bigevent.csproj b/src/tests/tracing/eventpipe/bigevent/bigevent.csproj new file mode 100644 index 0000000000000..04bd61f8dc87e --- /dev/null +++ b/src/tests/tracing/eventpipe/bigevent/bigevent.csproj @@ -0,0 +1,15 @@ + + + .NETCoreApp + exe + BuildAndRun + true + 0 + true + true + + + + + +