diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index c0d17c6dfd3f50..ac2720d00c10bd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs @@ -2703,6 +2703,14 @@ internal void DoCommand(EventCommandEventArgs commandArgs) foreach (int eventID in m_eventData.Keys) EnableEventForDispatcher(commandArgs.dispatcher, commandArgs.eventProviderType, eventID, IsEnabledByDefault(eventID, commandArgs.enable, commandArgs.level, commandArgs.matchAnyKeyword)); + // interpret perEventSourceSessionId's sign: a non-negative value indicates a session + // is being enabled, while a negative value (or 0 with enable=false) means disabled. + // Compute this before updating m_level/m_matchAnyKeyword so we can distinguish between + // a new session starting (expand filter) and a session stopping (replace filter). + bool bSessionEnable = (commandArgs.perEventSourceSessionId >= 0); + if (commandArgs.perEventSourceSessionId == 0 && !commandArgs.enable) + bSessionEnable = false; + if (commandArgs.enable) { if (!m_eventSourceEnabled) @@ -2711,9 +2719,9 @@ internal void DoCommand(EventCommandEventArgs commandArgs) m_level = commandArgs.level; m_matchAnyKeyword = commandArgs.matchAnyKeyword; } - else + else if (bSessionEnable) { - // Already enabled, make it the most verbose of the existing and new filter + // A new session is being added; expand the filter to cover the new session. if (commandArgs.level > m_level) m_level = commandArgs.level; if (commandArgs.matchAnyKeyword == 0) @@ -2721,14 +2729,15 @@ internal void DoCommand(EventCommandEventArgs commandArgs) else if (m_matchAnyKeyword != 0) m_matchAnyKeyword = unchecked(m_matchAnyKeyword | commandArgs.matchAnyKeyword); } + else + { + // A session is stopping but other sessions remain active; update the filter to + // the recomputed values which now reflect only the remaining sessions. + m_level = commandArgs.level; + m_matchAnyKeyword = commandArgs.matchAnyKeyword; + } } - // interpret perEventSourceSessionId's sign, and adjust perEventSourceSessionId to - // represent 0-based positive values - bool bSessionEnable = (commandArgs.perEventSourceSessionId >= 0); - if (commandArgs.perEventSourceSessionId == 0 && !commandArgs.enable) - bSessionEnable = false; - if (commandArgs.listener == null) { if (!bSessionEnable) diff --git a/src/native/eventpipe/ep-config.c b/src/native/eventpipe/ep-config.c index 6af68054447670..fdd93825927665 100644 --- a/src/native/eventpipe/ep-config.c +++ b/src/native/eventpipe/ep-config.c @@ -25,6 +25,7 @@ void config_compute_keyword_and_level ( const EventPipeConfiguration *config, const EventPipeProvider *provider, + const EventPipeSession *session_to_exclude, int64_t *keyword_for_all_sessions, EventPipeEventLevel *level_for_all_sessions); @@ -65,6 +66,7 @@ void config_compute_keyword_and_level ( const EventPipeConfiguration *config, const EventPipeProvider *provider, + const EventPipeSession *session_to_exclude, int64_t *keyword_for_all_sessions, EventPipeEventLevel *level_for_all_sessions) { @@ -80,7 +82,7 @@ config_compute_keyword_and_level ( for (int i = 0; i < EP_MAX_NUMBER_OF_SESSIONS; i++) { // Entering EventPipe lock gave us a barrier, we don't need more of them. EventPipeSession *session = ep_volatile_load_session_without_barrier (i); - if (session) { + if (session && session != session_to_exclude) { EventPipeSessionProviderList *providers = ep_session_get_providers (session); EP_ASSERT (providers != NULL); @@ -114,7 +116,7 @@ config_register_provider ( int64_t keyword_for_all_sessions; EventPipeEventLevel level_for_all_sessions; - config_compute_keyword_and_level (config, provider, &keyword_for_all_sessions, &level_for_all_sessions); + config_compute_keyword_and_level (config, provider, NULL, &keyword_for_all_sessions, &level_for_all_sessions); for (int i = 0; i < EP_MAX_NUMBER_OF_SESSIONS; i++) { // Entering EventPipe lock gave us a barrier, we don't need more of them. @@ -585,7 +587,9 @@ config_enable_disable ( EventPipeEventLevel level_for_all_sessions; EventPipeProviderCallbackData provider_callback_data; memset (&provider_callback_data, 0, sizeof (provider_callback_data)); - config_compute_keyword_and_level (config, provider, &keyword_for_all_sessions, &level_for_all_sessions); + // When disabling a session, exclude it from the combined keyword/level computation + // so that the computed values only reflect the remaining active sessions. + config_compute_keyword_and_level (config, provider, enable ? NULL : session, &keyword_for_all_sessions, &level_for_all_sessions); if (enable) { provider_set_config ( provider, diff --git a/src/tests/tracing/eventpipe/enabledisable/enabledisable.cs b/src/tests/tracing/eventpipe/enabledisable/enabledisable.cs index 0ab09ce6413337..1674c994780c17 100644 --- a/src/tests/tracing/eventpipe/enabledisable/enabledisable.cs +++ b/src/tests/tracing/eventpipe/enabledisable/enabledisable.cs @@ -127,4 +127,114 @@ private static void StopSession(EventPipeSession session, EventPipeEventSource s session.Stop(); } } + + [EventSource(Name = "Local.FilterRegressionEventSource")] + public sealed class FilterRegressionEventSource : EventSource + { + private readonly ConcurrentQueue _isEnabledOnDisable = new ConcurrentQueue(); + + public bool[] IsEnabledOnDisableResults => _isEnabledOnDisable.ToArray(); + + private FilterRegressionEventSource() { } + + public static FilterRegressionEventSource Log = new FilterRegressionEventSource(); + + [Event(1)] + public void TestEvent() { WriteEvent(1); } + + [NonEvent] + protected override void OnEventCommand(EventCommandEventArgs command) + { + base.OnEventCommand(command); + if (command.Command == EventCommand.Disable) + { + _isEnabledOnDisable.Enqueue(this.IsEnabled(EventLevel.Informational, (EventKeywords)0x2)); + } + } + } + + public class KeywordLevelFilterValidation + { + [ActiveIssue(" needs triage ", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoAnyAOT))] + [ActiveIssue(" needs triage ", TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] + [ActiveIssue("Can't find file dotnet-diagnostic-{pid}-*-socket", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsRiscv64Process))] + [Fact] + public static int TestEntryPoint() + { + // Verify that when one of multiple EventPipe sessions is stopped, the global keyword and level + // filters are recomputed to reflect only the remaining active sessions. + // + // Session 1 tracks keyword=0x2 at Informational level (the broader filter). + // Session 2 tracks keyword=0x1 at Error level (the narrower filter). + // After session 1 stops, only session 2 is active, so IsEnabled(Informational, keyword=0x2) + // must return false because neither the level (Error < Informational) nor keyword (0x1 & 0x2 == 0) + // matches session 2's configuration. + + // Force the EventSource to register with EventPipe before starting any sessions. If the + // type initializer has not run by the time a session starts, the provider callback won't fire. + FilterRegressionEventSource.Log.TestEvent(); + + var providers1 = new List + { + new EventPipeProvider("Local.FilterRegressionEventSource", EventLevel.Informational, 0x2) + }; + var providers2 = new List + { + new EventPipeProvider("Local.FilterRegressionEventSource", EventLevel.Error, 0x1) + }; + + DiagnosticsClient client = new DiagnosticsClient(Process.GetCurrentProcess().Id); +#if DIAGNOSTICS_RUNTIME + if (OperatingSystem.IsAndroid()) + client = new DiagnosticsClient(new IpcEndpointConfig("127.0.0.1:9000", IpcEndpointConfig.TransportType.TcpSocket, IpcEndpointConfig.PortType.Listen)); +#endif + + EventPipeSession session1 = client.StartEventPipeSession(providers1); + EventPipeEventSource source1 = new EventPipeEventSource(session1.EventStream); + + EventPipeSession session2 = client.StartEventPipeSession(providers2); + EventPipeEventSource source2 = new EventPipeEventSource(session2.EventStream); + + // Stop session1 (Informational, keyword=0x2) while session2 (Error, keyword=0x1) is still active. + // The global filter must be recomputed to only reflect session2's configuration. + StopSession(session1, source1); + StopSession(session2, source2); + + session1.Dispose(); + session2.Dispose(); + + bool[] results = FilterRegressionEventSource.Log.IsEnabledOnDisableResults; + + if (results.Length < 2) + { + Console.WriteLine($"Test failed: expected at least 2 Disable callbacks, got {results.Length}"); + return -1; + } + + // After session1 stops with session2 still active, the filter should reflect session2 only. + // IsEnabled(Informational, keyword=0x2) must be false: Error level (2) < Informational (4). + if (results[0]) + { + Console.WriteLine("Test failed: IsEnabled(Informational, keyword=0x2) should be false after the Informational session stops while the Error session remains active"); + return -1; + } + + // After all sessions stop, IsEnabled must still be false. + if (results[1]) + { + Console.WriteLine("Test failed: IsEnabled(Informational, keyword=0x2) should be false after all sessions stop"); + return -1; + } + + return 100; + } + + private static void StopSession(EventPipeSession session, EventPipeEventSource source) + { + source.Dynamic.All += _ => { }; + + Task.Run(source.Process); + session.Stop(); + } + } }