Issue correct Enable/Disable commands for EventSources with EventPipe (#81867)
authorDavid Mason <davmason@microsoft.com>
Fri, 3 Mar 2023 12:05:09 +0000 (04:05 -0800)
committerGitHub <noreply@github.com>
Fri, 3 Mar 2023 12:05:09 +0000 (04:05 -0800)
12 files changed:
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs
src/native/eventpipe/ep-config.c
src/native/eventpipe/ep-provider-internals.h
src/native/eventpipe/ep-provider.c
src/native/eventpipe/ep-types.h
src/native/eventpipe/ep.c
src/tests/issues.targets
src/tests/tracing/eventpipe/enabledisable/enabledisable.cs [new file with mode: 0644]
src/tests/tracing/eventpipe/enabledisable/enabledisable.csproj [new file with mode: 0644]

index a17e9a5..bef1e1f 100644 (file)
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Generic;
 using System.Runtime.InteropServices;
 
 namespace System.Diagnostics.Tracing
@@ -16,13 +17,56 @@ namespace System.Diagnostics.Tracing
             _eventProvider = new WeakReference<EventProvider>(eventProvider);
         }
 
+        protected override unsafe void HandleEnableNotification(
+                                    EventProvider target,
+                                    byte* additionalData,
+                                    byte level,
+                                    long matchAnyKeywords,
+                                    long matchAllKeywords,
+                                    Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData)
+        {
+            ulong id = 0;
+            if (additionalData != null)
+            {
+                id = BitConverter.ToUInt64(new ReadOnlySpan<byte>(additionalData, sizeof(ulong)));
+            }
+
+            // EventPipe issues Interop.Advapi32.EVENT_CONTROL_CODE_ENABLE_PROVIDER if a session
+            // is stopping as long as some other session is still enabled. If the session is stopping
+            // the session ID will be null, if it is a session starting it will be a non-zero value
+            bool bEnabling = id != 0;
+
+            IDictionary<string, string?>? args = null;
+            ControllerCommand command = ControllerCommand.Update;
+
+            if (bEnabling)
+            {
+                byte[]? filterDataBytes = null;
+                if (filterData != null)
+                {
+                    MarshalFilterData(filterData, out command, out filterDataBytes);
+                }
+
+                args = ParseFilterData(filterDataBytes);
+            }
+
+            // Since we are sharing logic across ETW and EventPipe in OnControllerCommand we have to set up data to
+            // mimic ETW to get the right commands sent to EventSources. perEventSourceSessionId has special meaning,
+            // if it is -1 the this command will be translated to a Disable command in EventSource.OnEventCommand. If
+            // it is 0-3 it indicates an ETW session with activities, and SessionMask.MAX (4) means legacy ETW session.
+            // We send SessionMask.MAX just to conform.
+            target.OnControllerCommand(command, args, bEnabling ? (int)SessionMask.MAX : -1);
+        }
+
         [UnmanagedCallersOnly]
         private static unsafe void Callback(byte* sourceId, int isEnabled, byte level,
             long matchAnyKeywords, long matchAllKeywords, Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData, void* callbackContext)
         {
             EventPipeEventProvider _this = (EventPipeEventProvider)GCHandle.FromIntPtr((IntPtr)callbackContext).Target!;
             if (_this._eventProvider.TryGetTarget(out EventProvider? target))
-                target.EnableCallback(isEnabled, level, matchAnyKeywords, matchAllKeywords, filterData);
+            {
+                _this.ProviderCallback(target, sourceId, isEnabled, level, matchAnyKeywords, matchAllKeywords, filterData);
+            }
         }
 
         // Register an event provider.
@@ -94,7 +138,7 @@ namespace System.Diagnostics.Tracing
 
         // Define an EventPipeEvent handle.
         internal override unsafe IntPtr DefineEventHandle(uint eventID, string eventName, long keywords, uint eventVersion, uint level,
-            byte *pMetadata, uint metadataLength)
+            bytepMetadata, uint metadataLength)
         {
             return EventPipeInternal.DefineEvent(_provHandle, eventID, keywords, eventVersion, level, pMetadata, metadataLength);
         }
index 51527b2..5753c4a 100644 (file)
@@ -35,6 +35,16 @@ namespace System.Diagnostics.Tracing
     /// <summary>
     /// Only here because System.Diagnostics.EventProvider needs one more extensibility hook (when it gets a
     /// controller callback)
+    ///
+    /// As of Feb 2023 the current factoring of this type remains a work in progress. Ideally all the ETW specific functionality
+    /// would be moved to EtwEventProvider and all the common functionality would be moved to EventProviderImpl. At that point this
+    /// type would no longer need to exist, EventSource would have a direct reference to EventProviderImpl, and EventProviderImpl's
+    /// WeakReference would point back to EventSource. However for now we still have this intermediate layer:
+    ///     EventSource -- EventProvider -- EventProviderImpl.
+    ///
+    /// Be careful interpreting code that uses 'ETW' naming.Some of it really is only used for ETW scenarios whereas in other places
+    /// ETW behavior was adopted as the standard that EventPipe also implements. Ideally the former would be moved to EtwEventProvider
+    /// and the latter would remove ETW from the naming.
     /// </summary>
     internal class EventProvider : IDisposable
     {
@@ -50,30 +60,10 @@ namespace System.Diagnostics.Tracing
             internal uint Reserved;
         }
 
-        /// <summary>
-        /// A struct characterizing ETW sessions (identified by the etwSessionId) as
-        /// activity-tracing-aware or legacy. A session that's activity-tracing-aware
-        /// has specified one non-zero bit in the reserved range 44-47 in the
-        /// 'allKeywords' value it passed in for a specific EventProvider.
-        /// </summary>
-        public struct SessionInfo
-        {
-            internal int sessionIdBit;      // the index of the bit used for tracing in the "reserved" field of AllKeywords
-            internal int etwSessionId;      // the machine-wide ETW session ID
-
-            internal SessionInfo(int sessionIdBit_, int etwSessionId_)
-            { sessionIdBit = sessionIdBit_; etwSessionId = etwSessionId_; }
-        }
-
-        internal EventProviderImpl m_eventProvider;      // The implementation of the specific logging mechanism functions.
-        private byte m_level;                            // Tracing Level
-        private long m_anyKeywordMask;                   // Trace Enable Flags
-        private long m_allKeywordMask;                   // Match all keyword
-        private List<SessionInfo>? m_liveSessions;       // current live sessions (KeyValuePair<sessionIdBit, etwSessionId>)
-        private bool m_enabled;                          // Enabled flag from Trace callback
-        private string? m_providerName;                  // Control name
-        private Guid m_providerId;                       // Control Guid
-        internal bool m_disposed;                        // when true provider has unregistered
+        internal EventProviderImpl _eventProvider;      // The implementation of the specific logging mechanism functions.
+        private string? _providerName;                  // Control name
+        private Guid _providerId;                       // Control Guid
+        internal bool _disposed;                        // when true provider has unregistered
 
         [ThreadStatic]
         private static WriteEventErrorCode s_returnCode; // The last return code
@@ -103,7 +93,7 @@ namespace System.Diagnostics.Tracing
         // EventSource has special logic to do this, no one else should be calling EventProvider.
         internal EventProvider(EventProviderType providerType)
         {
-            m_eventProvider = providerType switch
+            _eventProvider = providerType switch
             {
 #if TARGET_WINDOWS
                 EventProviderType.ETW => new EtwEventProvider(this),
@@ -122,10 +112,10 @@ namespace System.Diagnostics.Tracing
         /// </summary>
         internal unsafe void Register(EventSource eventSource)
         {
-            m_providerName = eventSource.Name;
-            m_providerId = eventSource.Guid;
+            _providerName = eventSource.Name;
+            _providerId = eventSource.Guid;
 
-            m_eventProvider.Register(eventSource);
+            _eventProvider.Register(eventSource);
         }
 
         //
@@ -152,21 +142,21 @@ namespace System.Diagnostics.Tracing
             //
             // check if the object has been already disposed
             //
-            if (m_disposed)
+            if (_disposed)
                 return;
 
             // Disable the provider.
-            m_enabled = false;
+            _eventProvider.Disable();
 
             // Do most of the work under a lock to avoid shutdown race.
 
             lock (EventListener.EventListenersLock)
             {
                 // Double check
-                if (m_disposed)
+                if (_disposed)
                     return;
 
-                m_disposed = true;
+                _disposed = true;
             }
 
             // We do the Unregistration outside the EventListenerLock because there is a lock
@@ -178,7 +168,7 @@ namespace System.Diagnostics.Tracing
             //
             // We solve by Unregistering after releasing the EventListenerLock.
             Debug.Assert(!Monitor.IsEntered(EventListener.EventListenersLock));
-            m_eventProvider.Unregister();
+            _eventProvider.Unregister();
         }
 
         /// <summary>
@@ -195,456 +185,93 @@ namespace System.Diagnostics.Tracing
             Dispose(false);
         }
 
-        internal unsafe void EnableCallback(
-                        int controlCode,
-                        byte setLevel,
-                        long anyKeyword,
-                        long allKeyword,
-                        Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData)
-        {
-            // This is an optional callback API. We will therefore ignore any failures that happen as a
-            // result of turning on this provider as to not crash the app.
-            // EventSource has code to validate whether initialization it expected to occur actually occurred
-            try
-            {
-                ControllerCommand command = ControllerCommand.Update;
-                IDictionary<string, string?>? args = null;
-                bool skipFinalOnControllerCommand = false;
-                if (controlCode == Interop.Advapi32.EVENT_CONTROL_CODE_ENABLE_PROVIDER)
-                {
-                    m_enabled = true;
-                    m_level = setLevel;
-                    m_anyKeywordMask = anyKeyword;
-                    m_allKeywordMask = allKeyword;
-
-                    List<KeyValuePair<SessionInfo, bool>> sessionsChanged = GetSessions();
-
-                    // The GetSessions() logic was here to support the idea that different ETW sessions
-                    // could have different user-defined filters.   (I believe it is currently broken but that is another matter.)
-                    // However in particular GetSessions() does not support EventPipe, only ETW, which is
-                    // the immediate problem.   We work-around establishing the invariant that we always get a
-                    // OnControllerCallback under all circumstances, even if we can't find a delta in the
-                    // ETW logic.  This fixes things for the EventPipe case.
-                    //
-                    // All this session based logic should be reviewed and likely removed, but that is a larger
-                    // change that needs more careful staging.
-                    if (sessionsChanged.Count == 0)
-                        sessionsChanged.Add(new KeyValuePair<SessionInfo, bool>(new SessionInfo(0, 0), true));
-
-                    foreach (KeyValuePair<SessionInfo, bool> session in sessionsChanged)
-                    {
-                        int sessionChanged = session.Key.sessionIdBit;
-                        int etwSessionId = session.Key.etwSessionId;
-                        bool bEnabling = session.Value;
-
-                        skipFinalOnControllerCommand = true;
-                        args = null;                                // reinitialize args for every session...
-
-                        // if we get more than one session changed we have no way
-                        // of knowing which one "filterData" belongs to
-                        if (sessionsChanged.Count > 1)
-                            filterData = null;
-
-                        // read filter data only when a session is being *added*
-                        if (bEnabling &&
-                            GetDataFromController(etwSessionId, filterData, out command, out byte[]? data, out int keyIndex))
-                        {
-                            args = new Dictionary<string, string?>(4);
-                            // data can be null if the filterArgs had a very large size which failed our sanity check
-                            if (data != null)
-                            {
-                                while (keyIndex < data.Length)
-                                {
-                                    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;
-                                }
-                            }
-                        }
-
-                        // execute OnControllerCommand once for every session that has changed.
-                        OnControllerCommand(command, args, bEnabling ? sessionChanged : -sessionChanged, etwSessionId);
-                    }
-                }
-                else if (controlCode == Interop.Advapi32.EVENT_CONTROL_CODE_DISABLE_PROVIDER)
-                {
-                    m_enabled = false;
-                    m_level = 0;
-                    m_anyKeywordMask = 0;
-                    m_allKeywordMask = 0;
-                    m_liveSessions = null;
-                }
-                else if (controlCode == Interop.Advapi32.EVENT_CONTROL_CODE_CAPTURE_STATE)
-                {
-                    command = ControllerCommand.SendManifest;
-                }
-                else
-                    return;     // per spec you ignore commands you don't recognize.
-
-                if (!skipFinalOnControllerCommand)
-                    OnControllerCommand(command, args, 0, 0);
-            }
-            catch
-            {
-                // We want to ignore any failures that happen as a result of turning on this provider as to
-                // not crash the app.
-            }
-        }
-
-        protected virtual void OnControllerCommand(ControllerCommand command, IDictionary<string, string?>? arguments, int sessionId, int etwSessionId) { }
+        internal virtual void OnControllerCommand(ControllerCommand command, IDictionary<string, string?>? arguments, int sessionId) { }
 
         protected EventLevel Level
         {
-            get => (EventLevel)m_level;
-            set => m_level = (byte)value;
+            get => _eventProvider.Level;
+            set => _eventProvider.Level = value;
         }
 
         protected EventKeywords MatchAnyKeyword
         {
-            get => (EventKeywords)m_anyKeywordMask;
-            set => m_anyKeywordMask = unchecked((long)value);
+            get => _eventProvider.MatchAnyKeyword;
+            set => _eventProvider.MatchAnyKeyword = value;
         }
 
         protected EventKeywords MatchAllKeyword
         {
-            get => (EventKeywords)m_allKeywordMask;
-            set => m_allKeywordMask = unchecked((long)value);
+            get => _eventProvider.MatchAllKeyword;
+            set => _eventProvider.MatchAllKeyword = value;
         }
 
-        private static int FindNull(byte[] buffer, int idx)
+        /// <summary>
+        /// IsEnabled, method used to test if provider is enabled
+        /// </summary>
+        public bool IsEnabled()
         {
-            while (idx < buffer.Length && buffer[idx] != 0)
-                idx++;
-            return idx;
+            return _eventProvider.IsEnabled();
         }
 
         /// <summary>
-        /// Determines the ETW sessions that have been added and/or removed to the set of
-        /// sessions interested in the current provider. It does so by (1) enumerating over all
-        /// ETW sessions that enabled 'this.m_Guid' for the current process ID, and (2)
-        /// comparing the current list with a list it cached on the previous invocation.
-        ///
-        /// The return value is a list of tuples, where the SessionInfo specifies the
-        /// ETW session that was added or remove, and the bool specifies whether the
-        /// session was added or whether it was removed from the set.
+        /// IsEnabled, method used to test if event is enabled
         /// </summary>
-        private List<KeyValuePair<SessionInfo, bool>> GetSessions()
+        /// <param name="level">
+        /// Level  to test
+        /// </param>
+        /// <param name="keywords">
+        /// Keyword  to test
+        /// </param>
+        public bool IsEnabled(byte level, long keywords)
         {
-            List<SessionInfo>? liveSessionList = null;
-
-            GetSessionInfo(
-                GetSessionInfoCallback,
-                ref liveSessionList);
-
-            List<KeyValuePair<SessionInfo, bool>> changedSessionList = new List<KeyValuePair<SessionInfo, bool>>();
-
-            // first look for sessions that have gone away (or have changed)
-            // (present in the m_liveSessions but not in the new liveSessionList)
-            if (m_liveSessions != null)
-            {
-                foreach (SessionInfo s in m_liveSessions)
-                {
-                    int idx;
-                    if ((idx = IndexOfSessionInList(liveSessionList, s.etwSessionId)) < 0 ||
-                        (liveSessionList![idx].sessionIdBit != s.sessionIdBit))
-                        changedSessionList.Add(new KeyValuePair<SessionInfo, bool>(s, false));
-                }
-            }
-            // next look for sessions that were created since the last callback  (or have changed)
-            // (present in the new liveSessionList but not in m_liveSessions)
-            if (liveSessionList != null)
-            {
-                foreach (SessionInfo s in liveSessionList)
-                {
-                    int idx;
-                    if ((idx = IndexOfSessionInList(m_liveSessions, s.etwSessionId)) < 0 ||
-                        (m_liveSessions![idx].sessionIdBit != s.sessionIdBit))
-                        changedSessionList.Add(new KeyValuePair<SessionInfo, bool>(s, true));
-                }
-            }
+            return _eventProvider.IsEnabled(level, keywords);
+        }
 
-            m_liveSessions = liveSessionList;
-            return changedSessionList;
+        public static WriteEventErrorCode GetLastWriteEventError()
+        {
+            return s_returnCode;
         }
 
-        /// <summary>
-        /// This method is the callback used by GetSessions() when it calls into GetSessionInfo().
-        /// It updates a List{SessionInfo} based on the etwSessionId and matchAllKeywords that
-        /// GetSessionInfo() passes in.
-        /// </summary>
-        private static void GetSessionInfoCallback(int etwSessionId, long matchAllKeywords,
-                                ref List<SessionInfo>? sessionList)
+        //
+        // Helper function to set the last error on the thread
+        //
+        private static void SetLastError(WriteEventErrorCode error)
         {
-            uint sessionIdBitMask = (uint)SessionMask.FromEventKeywords(unchecked((ulong)matchAllKeywords));
-            // an ETW controller that specifies more than the mandated bit for our EventSource
-            // will be ignored...
-            int val = BitOperations.PopCount(sessionIdBitMask);
-            if (val > 1)
-                return;
+            s_returnCode = error;
+        }
 
-            sessionList ??= new List<SessionInfo>(8);
+        private static unsafe object? EncodeObject(ref object? data, ref EventData* dataDescriptor, ref byte* dataBuffer, ref uint totalEventSize)
+        /*++
 
-            if (val == 1)
-            {
-                // activity-tracing-aware etw session
-                val = BitOperations.TrailingZeroCount(sessionIdBitMask);
-            }
-            else
-            {
-                // legacy etw session
-                val = BitOperations.PopCount((uint)SessionMask.All);
-            }
+        Routine Description:
 
-            sessionList.Add(new SessionInfo(val + 1, etwSessionId));
-        }
+           This routine is used by WriteEvent to unbox the object type and
+           to fill the passed in ETW data descriptor.
 
-        private delegate void SessionInfoCallback(int etwSessionId, long matchAllKeywords, ref List<SessionInfo>? sessionList);
+        Arguments:
 
-        /// <summary>
-        /// This method enumerates over all active ETW sessions that have enabled 'this.m_Guid'
-        /// for the current process ID, calling 'action' for each session, and passing it the
-        /// ETW session and the 'AllKeywords' the session enabled for the current provider.
-        /// </summary>
-        private
-#if !TARGET_WINDOWS
-        static
-#endif
-        unsafe void GetSessionInfo(SessionInfoCallback action, ref List<SessionInfo>? sessionList)
-        {
-#if TARGET_WINDOWS
-            int buffSize = 256;     // An initial guess that probably works most of the time.
-            byte* stackSpace = stackalloc byte[buffSize];
-            byte* buffer = stackSpace;
-            try
-            {
-                while (true)
-                {
-                    int hr = 0;
+           data - argument to be decoded
 
-                    fixed (Guid* provider = &m_providerId)
-                    {
-                        hr = Interop.Advapi32.EnumerateTraceGuidsEx(Interop.Advapi32.TRACE_QUERY_INFO_CLASS.TraceGuidQueryInfo,
-                            provider, sizeof(Guid), buffer, buffSize, out buffSize);
-                    }
-                    if (hr == 0)
-                        break;
-                    if (hr != Interop.Errors.ERROR_INSUFFICIENT_BUFFER)
-                        return;
+           dataDescriptor - pointer to the descriptor to be filled (updated to point to the next empty entry)
 
-                    if (buffer != stackSpace)
-                    {
-                        byte* toFree = buffer;
-                        buffer = null;
-                        Marshal.FreeHGlobal((IntPtr)toFree);
-                    }
-                    buffer = (byte*)Marshal.AllocHGlobal(buffSize);
-                }
+           dataBuffer - storage buffer for storing user data, needed because cant get the address of the object
+                        (updated to point to the next empty entry)
 
-                var providerInfos = (Interop.Advapi32.TRACE_GUID_INFO*)buffer;
-                var providerInstance = (Interop.Advapi32.TRACE_PROVIDER_INSTANCE_INFO*)&providerInfos[1];
-                int processId = unchecked((int)Interop.Kernel32.GetCurrentProcessId());
-                // iterate over the instances of the EventProvider in all processes
-                for (int i = 0; i < providerInfos->InstanceCount; i++)
-                {
-                    if (providerInstance->Pid == processId)
-                    {
-                        var enabledInfos = (Interop.Advapi32.TRACE_ENABLE_INFO*)&providerInstance[1];
-                        // iterate over the list of active ETW sessions "listening" to the current provider
-                        for (int j = 0; j < providerInstance->EnableCount; j++)
-                            action(enabledInfos[j].LoggerId, enabledInfos[j].MatchAllKeyword, ref sessionList);
-                    }
-                    if (providerInstance->NextOffset == 0)
-                        break;
-                    Debug.Assert(0 <= providerInstance->NextOffset && providerInstance->NextOffset < buffSize);
-                    byte* structBase = (byte*)providerInstance;
-                    providerInstance = (Interop.Advapi32.TRACE_PROVIDER_INSTANCE_INFO*)&structBase[providerInstance->NextOffset];
-                }
-            }
-            finally
-            {
-                if (buffer != null && buffer != stackSpace)
-                {
-                    Marshal.FreeHGlobal((IntPtr)buffer);
-                }
-            }
+        Return Value:
 
-#endif
-        }
+           null if the object is a basic type other than string or byte[]. String otherwise
 
-        /// <summary>
-        /// Returns the index of the SesisonInfo from 'sessions' that has the specified 'etwSessionId'
-        /// or -1 if the value is not present.
-        /// </summary>
-        private static int IndexOfSessionInList(List<SessionInfo>? sessions, int etwSessionId)
+        --*/
         {
-            if (sessions == null)
-                return -1;
-            // for non-coreclr code we could use List<T>.FindIndex(Predicate<T>), but we need this to compile
-            // on coreclr as well
-            for (int i = 0; i < sessions.Count; ++i)
-                if (sessions[i].etwSessionId == etwSessionId)
-                    return i;
+        Again:
+            dataDescriptor->Reserved = 0;
 
-            return -1;
-        }
+            string? sRet = data as string;
+            byte[]? blobRet = null;
 
-        /// <summary>
-        /// Gets any data to be passed from the controller to the provider.  It starts with what is passed
-        /// into the callback, but unfortunately this data is only present for when the provider is active
-        /// at the time the controller issues the command.  To allow for providers to activate after the
-        /// controller issued a command, we also check the registry and use that to get the data.  The function
-        /// returns an array of bytes representing the data, the index into that byte array where the data
-        /// starts, and the command being issued associated with that data.
-        /// </summary>
-        private
-#if !TARGET_WINDOWS
-        static
-#endif
-        unsafe bool GetDataFromController(int etwSessionId,
-            Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData, out ControllerCommand command, out byte[]? data, out int dataStart)
-        {
-            data = null;
-            dataStart = 0;
-            if (filterData == null)
+            if (sRet != null)
             {
-#if TARGET_WINDOWS
-                string regKey = @"\Microsoft\Windows\CurrentVersion\Winevt\Publishers\{" + m_providerId + "}";
-                if (IntPtr.Size == 8)
-                    regKey = @"Software\Wow6432Node" + regKey;
-                else
-                    regKey = "Software" + regKey;
-
-                string valueName = "ControllerData_Session_" + etwSessionId.ToString(CultureInfo.InvariantCulture);
-
-                // we need to assert this permission for partial trust scenarios
-                using (RegistryKey? key = Registry.LocalMachine.OpenSubKey(regKey))
-                {
-                    data = key?.GetValue(valueName, null) as byte[];
-                    if (data != null)
-                    {
-                        // We only used the persisted data from the registry for updates.
-                        command = ControllerCommand.Update;
-                        return true;
-                    }
-                }
-#endif
-            }
-            else
-            {
-                // 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)(void*)filterData->Ptr, data, 0, data.Length);
-                }
-                command = (ControllerCommand)filterData->Type;
-                return true;
-            }
-
-            command = ControllerCommand.Update;
-            return false;
-        }
-
-        /// <summary>
-        /// IsEnabled, method used to test if provider is enabled
-        /// </summary>
-        public bool IsEnabled()
-        {
-            return m_enabled;
-        }
-
-        /// <summary>
-        /// IsEnabled, method used to test if event is enabled
-        /// </summary>
-        /// <param name="level">
-        /// Level  to test
-        /// </param>
-        /// <param name="keywords">
-        /// Keyword  to test
-        /// </param>
-        public bool IsEnabled(byte level, long keywords)
-        {
-            //
-            // If not enabled at all, return false.
-            //
-            if (!m_enabled)
-            {
-                return false;
-            }
-
-            // This also covers the case of Level == 0.
-            if ((level <= m_level) ||
-                (m_level == 0))
-            {
-                //
-                // Check if Keyword is enabled
-                //
-
-                if ((keywords == 0) ||
-                    (((keywords & m_anyKeywordMask) != 0) &&
-                     ((keywords & m_allKeywordMask) == m_allKeywordMask)))
-                {
-                    return true;
-                }
-            }
-
-            return false;
-        }
-
-        public static WriteEventErrorCode GetLastWriteEventError()
-        {
-            return s_returnCode;
-        }
-
-        //
-        // Helper function to set the last error on the thread
-        //
-        private static void SetLastError(WriteEventErrorCode error)
-        {
-            s_returnCode = error;
-        }
-
-        private static unsafe object? EncodeObject(ref object? data, ref EventData* dataDescriptor, ref byte* dataBuffer, ref uint totalEventSize)
-        /*++
-
-        Routine Description:
-
-           This routine is used by WriteEvent to unbox the object type and
-           to fill the passed in ETW data descriptor.
-
-        Arguments:
-
-           data - argument to be decoded
-
-           dataDescriptor - pointer to the descriptor to be filled (updated to point to the next empty entry)
-
-           dataBuffer - storage buffer for storing user data, needed because cant get the address of the object
-                        (updated to point to the next empty entry)
-
-        Return Value:
-
-           null if the object is a basic type other than string or byte[]. String otherwise
-
-        --*/
-        {
-        Again:
-            dataDescriptor->Reserved = 0;
-
-            string? sRet = data as string;
-            byte[]? blobRet = null;
-
-            if (sRet != null)
-            {
-                dataDescriptor->Size = ((uint)sRet.Length + 1) * 2;
+                dataDescriptor->Size = ((uint)sRet.Length + 1) * 2;
             }
             else if ((blobRet = data as byte[]) != null)
             {
@@ -982,7 +609,7 @@ namespace System.Diagnostics.Tracing
                             userDataPtr[refObjPosition[7]].Ptr = (ulong)v7;
                         }
 
-                        status = m_eventProvider.EventWriteTransfer(in eventDescriptor, eventHandle, activityID, childActivityID, argCount, userData);
+                        status = _eventProvider.EventWriteTransfer(in eventDescriptor, eventHandle, activityID, childActivityID, argCount, userData);
                     }
                 }
                 else
@@ -1008,7 +635,7 @@ namespace System.Diagnostics.Tracing
                         }
                     }
 
-                    status = m_eventProvider.EventWriteTransfer(in eventDescriptor, eventHandle, activityID, childActivityID, argCount, userData);
+                    status = _eventProvider.EventWriteTransfer(in eventDescriptor, eventHandle, activityID, childActivityID, argCount, userData);
 
                     for (int i = 0; i < refObjIndex; ++i)
                     {
@@ -1074,7 +701,7 @@ namespace System.Diagnostics.Tracing
                                 (EventOpcode)eventDescriptor.Opcode == EventOpcode.Stop);
             }
 
-            WriteEventErrorCode status = m_eventProvider.EventWriteTransfer(in eventDescriptor, eventHandle, activityID, childActivityID, dataCount, (EventData*)data);
+            WriteEventErrorCode status = _eventProvider.EventWriteTransfer(in eventDescriptor, eventHandle, activityID, childActivityID, dataCount, (EventData*)data);
 
             if (status != 0)
             {
@@ -1092,7 +719,7 @@ namespace System.Diagnostics.Tracing
             int dataCount,
             IntPtr data)
         {
-            WriteEventErrorCode status = m_eventProvider.EventWriteTransfer(
+            WriteEventErrorCode status = _eventProvider.EventWriteTransfer(
                 in eventDescriptor,
                 eventHandle,
                 activityID,
@@ -1115,7 +742,7 @@ namespace System.Diagnostics.Tracing
             void* data,
             uint dataSize)
         {
-            return ((EtwEventProvider)m_eventProvider).SetInformation(eventInfoClass, data, dataSize);
+            return ((EtwEventProvider)_eventProvider).SetInformation(eventInfoClass, data, dataSize);
         }
 #endif
     }
@@ -1124,15 +751,88 @@ namespace System.Diagnostics.Tracing
     // A wrapper around the ETW-specific API calls.
     internal sealed class EtwEventProvider : EventProviderImpl
     {
+        /// <summary>
+        /// A struct characterizing ETW sessions (identified by the etwSessionId) as
+        /// activity-tracing-aware or legacy. A session that's activity-tracing-aware
+        /// has specified one non-zero bit in the reserved range 44-47 in the
+        /// 'allKeywords' value it passed in for a specific EventProvider.
+        /// </summary>
+        public struct SessionInfo
+        {
+            internal int sessionIdBit;      // the index of the bit used for tracing in the "reserved" field of AllKeywords
+            internal int etwSessionId;      // the machine-wide ETW session ID
+
+            internal SessionInfo(int sessionIdBit_, int etwSessionId_)
+            { sessionIdBit = sessionIdBit_; etwSessionId = etwSessionId_; }
+        }
+
         private readonly WeakReference<EventProvider> _eventProvider;
         private long _registrationHandle;
         private GCHandle _gcHandle;
+        private List<SessionInfo>? _liveSessions;       // current live sessions (KeyValuePair<sessionIdBit, etwSessionId>)
+        private Guid _providerId;
 
         internal EtwEventProvider(EventProvider eventProvider)
         {
             _eventProvider = new WeakReference<EventProvider>(eventProvider);
         }
 
+        internal override void Disable()
+        {
+            base.Disable();
+            _liveSessions = null;
+        }
+
+        protected override unsafe void HandleEnableNotification(
+                                    EventProvider target,
+                                    byte *additionalData,
+                                    byte level,
+                                    long matchAnyKeywords,
+                                    long matchAllKeywords,
+                                    Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData)
+        {
+            Debug.Assert(additionalData == null);
+
+            // The GetSessions() logic was here to support the idea that different ETW sessions
+            // could have different user-defined filters. (I believe it is currently broken.)
+            //
+            // All this session based logic should be reviewed and likely removed, but that is a larger
+            // change that needs more careful staging.
+            List<KeyValuePair<SessionInfo, bool>> sessionsChanged = GetChangedSessions();
+
+            foreach (KeyValuePair<SessionInfo, bool> session in sessionsChanged)
+            {
+                int sessionChanged = session.Key.sessionIdBit;
+                bool bEnabling = session.Value;
+
+                // reinitialize args for every session...
+                IDictionary<string, string?>? args = null;
+                ControllerCommand command = ControllerCommand.Update;
+
+                // read filter data only when a session is being *added*
+                if (bEnabling)
+                {
+                    byte[]? filterDataBytes;
+                    // if we get more than one session changed we have no way
+                    // of knowing which one "filterData" belongs to
+                    if (sessionsChanged.Count > 1 || filterData == null)
+                    {
+                        TryReadRegistryFilterData(session.Key.etwSessionId, out command, out filterDataBytes);
+                    }
+                    else
+                    {
+                        MarshalFilterData(filterData, out command, out filterDataBytes);
+                    }
+                    args = ParseFilterData(filterDataBytes);
+                }
+
+                // execute OnControllerCommand once for every session that has changed.
+                // If the sessionId argument is positive it will be sent to the EventSource as an Enable,
+                // and if it is negative it will be sent as a disable. See EventSource.DoCommand()
+                target.OnControllerCommand(command, args, bEnabling ? sessionChanged : -sessionChanged);
+            }
+        }
+
         [UnmanagedCallersOnly]
         private static unsafe void Callback(Guid* sourceId, int isEnabled, byte level,
             long matchAnyKeywords, long matchAllKeywords, Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData, void* callbackContext)
@@ -1140,9 +840,12 @@ namespace System.Diagnostics.Tracing
             EtwEventProvider _this = (EtwEventProvider)GCHandle.FromIntPtr((IntPtr)callbackContext).Target!;
 
             if (_this._eventProvider.TryGetTarget(out EventProvider? target))
-                target.EnableCallback(isEnabled, level, matchAnyKeywords, matchAllKeywords, filterData);
+            {
+                _this.ProviderCallback(target, null, isEnabled, level, matchAnyKeywords, matchAllKeywords, filterData);
+            }
         }
 
+
         // Register an event provider.
         internal override unsafe void Register(EventSource eventSource)
         {
@@ -1150,7 +853,8 @@ namespace System.Diagnostics.Tracing
             _gcHandle = GCHandle.Alloc(this);
 
             long registrationHandle = 0;
-            Guid providerId = eventSource.Guid;
+            _providerId = eventSource.Guid;
+            Guid providerId = _providerId;
             uint status = Interop.Advapi32.EventRegister(
                 &providerId,
                 &Callback,
@@ -1161,6 +865,7 @@ namespace System.Diagnostics.Tracing
                 _gcHandle.Free();
                 throw new ArgumentException(Interop.Kernel32.GetMessage((int)status));
             }
+
             Debug.Assert(_registrationHandle == 0);
             _registrationHandle = registrationHandle;
         }
@@ -1173,6 +878,7 @@ namespace System.Diagnostics.Tracing
                 Interop.Advapi32.EventUnregister(_registrationHandle);
                 _registrationHandle = 0;
             }
+
             if (_gcHandle.IsAllocated)
             {
                 _gcHandle.Free();
@@ -1251,13 +957,308 @@ namespace System.Diagnostics.Tracing
 
             return status;
         }
+
+        /// <summary>
+        /// Callback data for ETW is only present when the provider is active at the time the controller issues the command.
+        /// To allow for providers to activate after the controller issued a command, we also check the registry and use that
+        /// to get the data. The function returns an array of bytes representing the data, the index into that byte array
+        /// where the data starts, and the command being issued associated with that data.
+        /// </summary>
+        private unsafe bool TryReadRegistryFilterData(int etwSessionId, out ControllerCommand command, out byte[]? data)
+        {
+            command = ControllerCommand.Update;
+            data = null;
+
+            string regKey = @"\Microsoft\Windows\CurrentVersion\Winevt\Publishers\{" + _providerId + "}";
+            if (IntPtr.Size == 8)
+            {
+                regKey = @"Software\Wow6432Node" + regKey;
+            }
+            else
+            {
+                regKey = "Software" + regKey;
+            }
+
+            string valueName = "ControllerData_Session_" + etwSessionId.ToString(CultureInfo.InvariantCulture);
+
+            // we need to assert this permission for partial trust scenarios
+            using (RegistryKey? key = Registry.LocalMachine.OpenSubKey(regKey))
+            {
+                data = key?.GetValue(valueName, null) as byte[];
+                if (data != null)
+                {
+                    // We only used the persisted data from the registry for updates.
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        /// <summary>
+        /// Determines the ETW sessions that have been added and/or removed to the set of
+        /// sessions interested in the current provider. It does so by (1) enumerating over all
+        /// ETW sessions that enabled 'this.m_Guid' for the current process ID, and (2)
+        /// comparing the current list with a list it cached on the previous invocation.
+        ///
+        /// The return value is a list of tuples, where the SessionInfo specifies the
+        /// ETW session that was added or remove, and the bool specifies whether the
+        /// session was added or whether it was removed from the set.
+        /// </summary>
+        private List<KeyValuePair<SessionInfo, bool>> GetChangedSessions()
+        {
+            List<SessionInfo>? liveSessionList = null;
+
+            GetSessionInfo(
+                GetSessionInfoCallback,
+                ref liveSessionList);
+
+            List<KeyValuePair<SessionInfo, bool>> changedSessionList = new List<KeyValuePair<SessionInfo, bool>>();
+
+            // first look for sessions that have gone away (or have changed)
+            // (present in the _liveSessions but not in the new liveSessionList)
+            if (_liveSessions != null)
+            {
+                foreach (SessionInfo s in _liveSessions)
+                {
+                    int idx;
+                    if ((idx = IndexOfSessionInList(liveSessionList, s.etwSessionId)) < 0 ||
+                        (liveSessionList![idx].sessionIdBit != s.sessionIdBit))
+                        changedSessionList.Add(new KeyValuePair<SessionInfo, bool>(s, false));
+                }
+            }
+            // next look for sessions that were created since the last callback  (or have changed)
+            // (present in the new liveSessionList but not in _liveSessions)
+            if (liveSessionList != null)
+            {
+                foreach (SessionInfo s in liveSessionList)
+                {
+                    int idx;
+                    if ((idx = IndexOfSessionInList(_liveSessions, s.etwSessionId)) < 0 ||
+                        (_liveSessions![idx].sessionIdBit != s.sessionIdBit))
+                        changedSessionList.Add(new KeyValuePair<SessionInfo, bool>(s, true));
+                }
+            }
+
+            _liveSessions = liveSessionList;
+            return changedSessionList;
+        }
+
+        /// <summary>
+        /// This method is the callback used by GetSessions() when it calls into GetSessionInfo().
+        /// It updates a List{SessionInfo} based on the etwSessionId and matchAllKeywords that
+        /// GetSessionInfo() passes in.
+        /// </summary>
+        private static void GetSessionInfoCallback(int etwSessionId, long matchAllKeywords,
+                                ref List<SessionInfo>? sessionList)
+        {
+            uint sessionIdBitMask = (uint)SessionMask.FromEventKeywords(unchecked((ulong)matchAllKeywords));
+            // an ETW controller that specifies more than the mandated bit for our EventSource
+            // will be ignored...
+            int val = BitOperations.PopCount(sessionIdBitMask);
+            if (val > 1)
+                return;
+
+            sessionList ??= new List<SessionInfo>(8);
+
+            if (val == 1)
+            {
+                // activity-tracing-aware etw session
+                val = BitOperations.TrailingZeroCount(sessionIdBitMask);
+            }
+            else
+            {
+                // legacy etw session
+                val = BitOperations.PopCount((uint)SessionMask.All);
+            }
+
+            sessionList.Add(new SessionInfo(val + 1, etwSessionId));
+        }
+
+        private delegate void SessionInfoCallback(int etwSessionId, long matchAllKeywords, ref List<SessionInfo>? sessionList);
+
+        /// <summary>
+        /// This method enumerates over all active ETW sessions that have enabled 'this.m_Guid'
+        /// for the current process ID, calling 'action' for each session, and passing it the
+        /// ETW session and the 'AllKeywords' the session enabled for the current provider.
+        /// </summary>
+        private unsafe void GetSessionInfo(SessionInfoCallback action, ref List<SessionInfo>? sessionList)
+        {
+            int buffSize = 256;     // An initial guess that probably works most of the time.
+            byte* stackSpace = stackalloc byte[buffSize];
+            byte* buffer = stackSpace;
+            try
+            {
+                while (true)
+                {
+                    int hr = 0;
+
+                    fixed (Guid* provider = &_providerId)
+                    {
+                        hr = Interop.Advapi32.EnumerateTraceGuidsEx(Interop.Advapi32.TRACE_QUERY_INFO_CLASS.TraceGuidQueryInfo,
+                            provider, sizeof(Guid), buffer, buffSize, out buffSize);
+                    }
+                    if (hr == 0)
+                        break;
+                    if (hr != Interop.Errors.ERROR_INSUFFICIENT_BUFFER)
+                        return;
+
+                    if (buffer != stackSpace)
+                    {
+                        byte* toFree = buffer;
+                        buffer = null;
+                        Marshal.FreeHGlobal((IntPtr)toFree);
+                    }
+                    buffer = (byte*)Marshal.AllocHGlobal(buffSize);
+                }
+
+                var providerInfos = (Interop.Advapi32.TRACE_GUID_INFO*)buffer;
+                var providerInstance = (Interop.Advapi32.TRACE_PROVIDER_INSTANCE_INFO*)&providerInfos[1];
+                int processId = unchecked((int)Interop.Kernel32.GetCurrentProcessId());
+                // iterate over the instances of the EventProvider in all processes
+                for (int i = 0; i < providerInfos->InstanceCount; i++)
+                {
+                    if (providerInstance->Pid == processId)
+                    {
+                        var enabledInfos = (Interop.Advapi32.TRACE_ENABLE_INFO*)&providerInstance[1];
+                        // iterate over the list of active ETW sessions "listening" to the current provider
+                        for (int j = 0; j < providerInstance->EnableCount; j++)
+                            action(enabledInfos[j].LoggerId, enabledInfos[j].MatchAllKeyword, ref sessionList);
+                    }
+                    if (providerInstance->NextOffset == 0)
+                        break;
+                    Debug.Assert(0 <= providerInstance->NextOffset && providerInstance->NextOffset < buffSize);
+                    byte* structBase = (byte*)providerInstance;
+                    providerInstance = (Interop.Advapi32.TRACE_PROVIDER_INSTANCE_INFO*)&structBase[providerInstance->NextOffset];
+                }
+            }
+            finally
+            {
+                if (buffer != null && buffer != stackSpace)
+                {
+                    Marshal.FreeHGlobal((IntPtr)buffer);
+                }
+            }
+        }
+
+        /// <summary>
+        /// Returns the index of the SesisonInfo from 'sessions' that has the specified 'etwSessionId'
+        /// or -1 if the value is not present.
+        /// </summary>
+        private static int IndexOfSessionInList(List<SessionInfo>? sessions, int etwSessionId)
+        {
+            if (sessions == null)
+                return -1;
+            // for non-coreclr code we could use List<T>.FindIndex(Predicate<T>), but we need this to compile
+            // on coreclr as well
+            for (int i = 0; i < sessions.Count; ++i)
+                if (sessions[i].etwSessionId == etwSessionId)
+                    return i;
+
+            return -1;
+        }
+
     }
 #endif
 
 #pragma warning disable CA1852 // EventProviderImpl is not derived from in all targets
-
     internal class EventProviderImpl
     {
+        protected byte _level;                            // Tracing Level
+        protected long _anyKeywordMask;                   // Trace Enable Flags
+        protected long _allKeywordMask;                   // Match all keyword
+        protected bool _enabled;                          // Enabled flag from Trace callback
+
+        internal EventLevel Level
+        {
+            get => (EventLevel)_level;
+            set => _level = (byte)value;
+        }
+
+        internal EventKeywords MatchAnyKeyword
+        {
+            get => (EventKeywords)_anyKeywordMask;
+            set => _anyKeywordMask = unchecked((long)value);
+        }
+
+        internal EventKeywords MatchAllKeyword
+        {
+            get => (EventKeywords)_allKeywordMask;
+            set => _allKeywordMask = unchecked((long)value);
+        }
+
+        protected virtual unsafe void HandleEnableNotification(
+                                    EventProvider target,
+                                    byte *additionalData,
+                                    byte level,
+                                    long matchAnyKeywords,
+                                    long matchAllKeywords,
+                                    Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData)
+        {
+        }
+
+        /// <summary>
+        /// IsEnabled, method used to test if provider is enabled
+        /// </summary>
+        public bool IsEnabled()
+        {
+            return _enabled;
+        }
+
+        /// <summary>
+        /// IsEnabled, method used to test if event is enabled
+        /// </summary>
+        /// <param name="level">
+        /// Level  to test
+        /// </param>
+        /// <param name="keywords">
+        /// Keyword  to test
+        /// </param>
+        public bool IsEnabled(byte level, long keywords)
+        {
+            //
+            // If not enabled at all, return false.
+            //
+            if (!_enabled)
+            {
+                return false;
+            }
+
+            // This also covers the case of Level == 0.
+            if ((level <= _level) ||
+                (_level == 0))
+            {
+                //
+                // Check if Keyword is enabled
+                //
+
+                if ((keywords == 0) ||
+                    (((keywords & _anyKeywordMask) != 0) &&
+                     ((keywords & _allKeywordMask) == _allKeywordMask)))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        internal void Enable(byte level, long anyKeyword, long allKeyword)
+        {
+            _enabled = true;
+            _level = level;
+            _anyKeywordMask = anyKeyword;
+            _allKeywordMask = allKeyword;
+        }
+
+        internal virtual void Disable()
+        {
+            _enabled = false;
+            _level = 0;
+            _anyKeywordMask = 0;
+            _allKeywordMask = 0;
+        }
+
         internal virtual void Register(EventSource eventSource)
         {
         }
@@ -1288,8 +1289,104 @@ namespace System.Diagnostics.Tracing
         {
             return IntPtr.Zero;
         }
-    }
 
-#pragma warning restore CA1852
+        protected unsafe void ProviderCallback(
+                        EventProvider target,
+                        byte *additionalData,
+                        int controlCode,
+                        byte level,
+                        long matchAnyKeywords,
+                        long matchAllKeywords,
+                        Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData)
+        {
+            // This is an optional callback API. We will therefore ignore any failures that happen as a
+            // result of turning on this provider as to not crash the app.
+            // EventSource has code to validate whether initialization it expected to occur actually occurred
+            try
+            {
+                if (controlCode == Interop.Advapi32.EVENT_CONTROL_CODE_ENABLE_PROVIDER)
+                {
+                    Enable(level, matchAnyKeywords, matchAllKeywords);
+                    HandleEnableNotification(target, additionalData, level, matchAnyKeywords, matchAllKeywords, filterData);
+                    return;
+                }
+
+                ControllerCommand command = ControllerCommand.Update;
+                if (controlCode == Interop.Advapi32.EVENT_CONTROL_CODE_DISABLE_PROVIDER)
+                {
+                    Disable();
+                }
+                else if (controlCode == Interop.Advapi32.EVENT_CONTROL_CODE_CAPTURE_STATE)
+                {
+                    command = ControllerCommand.SendManifest;
+                }
+                else
+                {
+                    return;     // per spec you ignore commands you don't recognize.
+                }
+
+                target.OnControllerCommand(command, null, 0);
+            }
+            catch
+            {
+                // We want to ignore any failures that happen as a result of turning on this provider as to
+                // not crash the app.
+            }
+        }
+
+        private static int FindNull(byte[] buffer, int idx)
+        {
+            while (idx < buffer.Length && buffer[idx] != 0)
+            {
+                idx++;
+            }
+
+            return idx;
+        }
 
+        protected static unsafe IDictionary<string, string?>? ParseFilterData(byte[]? data)
+        {
+            IDictionary<string, string?>? args = null;
+
+            // data can be null if the filterArgs had a very large size which failed our sanity check
+            if (data != null)
+            {
+                args = new Dictionary<string, string?>(4);
+                int dataStart = 0;
+                while (dataStart < data.Length)
+                {
+                    int keyEnd = FindNull(data, dataStart);
+                    int valueIdx = keyEnd + 1;
+                    int valueEnd = FindNull(data, valueIdx);
+                    if (valueEnd < data.Length)
+                    {
+                        string key = System.Text.Encoding.UTF8.GetString(data, dataStart, keyEnd - dataStart);
+                        string value = System.Text.Encoding.UTF8.GetString(data, valueIdx, valueEnd - valueIdx);
+                        args[key] = value;
+                    }
+                    dataStart = valueEnd + 1;
+                }
+            }
+
+            return args;
+        }
+
+        protected unsafe bool MarshalFilterData(Interop.Advapi32.EVENT_FILTER_DESCRIPTOR* filterData, out ControllerCommand command, out byte[]? data)
+        {
+            Debug.Assert(filterData != null);
+
+            data = null;
+            // 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)(void*)filterData->Ptr, data, 0, data.Length);
+            }
+            command = (ControllerCommand)filterData->Type;
+            return true;
+        }
+    }
+#pragma warning restore CA1852
 }
index 52fe352..947d344 100644 (file)
@@ -220,16 +220,6 @@ namespace System.Diagnostics.Tracing
     // The EnsureDescriptorsInitialized() method might need to access EventSource and its derived type
     // members and the trimmer ensures that these members are preserved.
     [DynamicallyAccessedMembers(ManifestMemberTypes)]
-    [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2113:ReflectionToRequiresUnreferencedCode",
-        Justification = "EnsureDescriptorsInitialized's use of GetType preserves methods on Delegate and MulticastDelegate " +
-                        "because the nested type OverrideEventProvider's base type EventProvider defines a delegate. " +
-                        "This includes Delegate and MulticastDelegate methods which require unreferenced code, but " +
-                        "EnsureDescriptorsInitialized does not access these members and is safe to call.")]
-    [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2115:ReflectionToDynamicallyAccessedMembers",
-        Justification = "EnsureDescriptorsInitialized's use of GetType preserves methods on Delegate and MulticastDelegate " +
-                        "because the nested type OverrideEventProvider's base type EventProvider defines a delegate. " +
-                        "This includes Delegate and MulticastDelegate methods which have dynamically accessed members requirements, but " +
-                        "EnsureDescriptorsInitialized does not access these members and is safe to call.")]
     public partial class EventSource : IDisposable
     {
 
@@ -448,7 +438,7 @@ namespace System.Diagnostics.Tracing
                 throw new ArgumentException(SR.EventSource_InvalidCommand, nameof(command));
             }
 
-            eventSource.SendCommand(null, EventProviderType.ETW, 0, 0, command, true, EventLevel.LogAlways, EventKeywords.None, commandArguments);
+            eventSource.SendCommand(null, EventProviderType.ETW, 0, command, true, EventLevel.LogAlways, EventKeywords.None, commandArguments);
         }
 
         // Error APIs.  (We don't throw by default, but you can probe for status)
@@ -745,7 +735,7 @@ namespace System.Diagnostics.Tracing
 
                 fixed (byte *pMetadata = metadata)
                 {
-                    IntPtr eventHandle = m_eventPipeProvider.m_eventProvider.DefineEventHandle(
+                    IntPtr eventHandle = m_eventPipeProvider._eventProvider.DefineEventHandle(
                         eventID,
                         eventName,
                         keywords,
@@ -2168,7 +2158,7 @@ namespace System.Diagnostics.Tracing
 
                                     fixed (byte* pMetadata = metadata)
                                     {
-                                        m_writeEventStringEventHandle = m_eventPipeProvider.m_eventProvider.DefineEventHandle(0, eventName, keywords, 0, (uint)level,
+                                        m_writeEventStringEventHandle = m_eventPipeProvider._eventProvider.DefineEventHandle(0, eventName, keywords, 0, (uint)level,
                                                                             pMetadata, metadataLength);
                                     }
                                 }
@@ -2367,12 +2357,12 @@ namespace System.Diagnostics.Tracing
                 this.m_eventSource = eventSource;
                 this.m_eventProviderType = providerType;
             }
-            protected override void OnControllerCommand(ControllerCommand command, IDictionary<string, string?>? arguments,
-                                                              int perEventSourceSessionId, int etwSessionId)
+            internal override void OnControllerCommand(ControllerCommand command, IDictionary<string, string?>? arguments,
+                                                              int perEventSourceSessionId)
             {
                 // We use null to represent the ETW EventListener.
                 EventListener? listener = null;
-                m_eventSource.SendCommand(listener, m_eventProviderType, perEventSourceSessionId, etwSessionId,
+                m_eventSource.SendCommand(listener, m_eventProviderType, perEventSourceSessionId,
                                           (EventCommand)command, IsEnabled(), Level, MatchAnyKeyword, arguments);
             }
             private readonly EventSource m_eventSource;
@@ -2490,7 +2480,7 @@ namespace System.Diagnostics.Tracing
         //     * The 'enabled' 'level', matchAnyKeyword' arguments are ignored (must be true, 0, 0).
         //
         // dispatcher == null has special meaning. It is the 'ETW' dispatcher.
-        internal void SendCommand(EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId, int etwSessionId,
+        internal void SendCommand(EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId,
                                   EventCommand command, bool enable,
                                   EventLevel level, EventKeywords matchAnyKeyword,
                                   IDictionary<string, string?>? commandArguments)
@@ -2500,7 +2490,7 @@ namespace System.Diagnostics.Tracing
                 return;
             }
 
-            var commandArgs = new EventCommandEventArgs(command, commandArguments, this, listener, eventProviderType, perEventSourceSessionId, etwSessionId, enable, level, matchAnyKeyword);
+            var commandArgs = new EventCommandEventArgs(command, commandArguments, this, listener, eventProviderType, perEventSourceSessionId, enable, level, matchAnyKeyword);
             lock (EventListener.EventListenersLock)
             {
                 if (m_completelyInited)
@@ -4058,7 +4048,7 @@ namespace System.Diagnostics.Tracing
         {
             ArgumentNullException.ThrowIfNull(eventSource);
 
-            eventSource.SendCommand(this, EventProviderType.None, 0, 0, EventCommand.Update, true, level, matchAnyKeyword, arguments);
+            eventSource.SendCommand(this, EventProviderType.None, 0, EventCommand.Update, true, level, matchAnyKeyword, arguments);
 
 #if FEATURE_PERFTRACING
             if (eventSource.GetType() == typeof(NativeRuntimeEventSource))
@@ -4076,7 +4066,7 @@ namespace System.Diagnostics.Tracing
         {
             ArgumentNullException.ThrowIfNull(eventSource);
 
-            eventSource.SendCommand(this, EventProviderType.None, 0, 0, EventCommand.Update, false, EventLevel.LogAlways, EventKeywords.None, null);
+            eventSource.SendCommand(this, EventProviderType.None, 0, EventCommand.Update, false, EventLevel.LogAlways, EventKeywords.None, null);
 
 #if FEATURE_PERFTRACING
             if (eventSource.GetType() == typeof(NativeRuntimeEventSource))
@@ -4505,7 +4495,7 @@ namespace System.Diagnostics.Tracing
 #region private
 
         internal EventCommandEventArgs(EventCommand command, IDictionary<string, string?>? arguments, EventSource eventSource,
-            EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId, int etwSessionId, bool enable, EventLevel level, EventKeywords matchAnyKeyword)
+            EventListener? listener, EventProviderType eventProviderType, int perEventSourceSessionId, bool enable, EventLevel level, EventKeywords matchAnyKeyword)
         {
             this.Command = command;
             this.Arguments = arguments;
@@ -4513,7 +4503,6 @@ namespace System.Diagnostics.Tracing
             this.listener = listener;
             this.eventProviderType = eventProviderType;
             this.perEventSourceSessionId = perEventSourceSessionId;
-            this.etwSessionId = etwSessionId;
             this.enable = enable;
             this.level = level;
             this.matchAnyKeyword = matchAnyKeyword;
@@ -4526,7 +4515,6 @@ namespace System.Diagnostics.Tracing
         // These are the arguments of sendCommand and are only used for deferring commands until after we are fully initialized.
         internal EventListener? listener;
         internal int perEventSourceSessionId;
-        internal int etwSessionId;
         internal bool enable;
         internal EventLevel level;
         internal EventKeywords matchAnyKeyword;
index 8b61c04..2f312f9 100644 (file)
@@ -95,7 +95,7 @@ namespace System.Diagnostics.Tracing
                             fixed (byte* pMetadataBlob = metadata)
                             {
                                 // Define the event.
-                                eventHandle = provider.m_eventProvider.DefineEventHandle(
+                                eventHandle = provider._eventProvider.DefineEventHandle(
                                     (uint)descriptor.EventId,
                                     name,
                                     descriptor.Keywords,
index 7199650..c8abf11 100644 (file)
@@ -122,7 +122,8 @@ config_register_provider (
                                        ep_session_provider_get_keywords (session_provider),
                                        ep_session_provider_get_logging_level (session_provider),
                                        ep_session_provider_get_filter_data (session_provider),
-                                       &provider_callback_data);
+                                       &provider_callback_data,
+                                       (EventPipeSessionID)session);
                                if (provider_callback_data_queue)
                                        ep_provider_callback_data_queue_enqueue (provider_callback_data_queue, &provider_callback_data);
                                ep_provider_callback_data_fini (&provider_callback_data);
@@ -562,7 +563,8 @@ config_enable_disable (
                                                        ep_session_provider_get_keywords (session_provider),
                                                        ep_session_provider_get_logging_level (session_provider),
                                                        ep_session_provider_get_filter_data (session_provider),
-                                                       &provider_callback_data);
+                                                       &provider_callback_data,
+                                                       (EventPipeSessionID)session);
                                        } else {
                                                provider_unset_config (
                                                        provider,
index 23bf2cb..a4f2a25 100644 (file)
@@ -21,7 +21,8 @@ provider_set_config (
        int64_t keywords,
        EventPipeEventLevel level,
        const ep_char8_t *filter_data,
-       EventPipeProviderCallbackData *callback_data);
+       EventPipeProviderCallbackData *callback_data,
+       EventPipeSessionID session_id);
 
 // Unset the provider configuration for the specified session (disable sets of events).
 // _Requires_lock_held (ep)
index cd7d551..5939fd6 100644 (file)
@@ -28,7 +28,8 @@ provider_prepare_callback_data (
        int64_t keywords,
        EventPipeEventLevel provider_level,
        const ep_char8_t *filter_data,
-       EventPipeProviderCallbackData *provider_callback_data);
+       EventPipeProviderCallbackData *provider_callback_data,
+       EventPipeSessionID session_id);
 
 // _Requires_lock_held (ep)
 static
@@ -69,7 +70,8 @@ provider_prepare_callback_data (
        int64_t keywords,
        EventPipeEventLevel provider_level,
        const ep_char8_t *filter_data,
-       EventPipeProviderCallbackData *provider_callback_data)
+       EventPipeProviderCallbackData *provider_callback_data,
+       EventPipeSessionID session_id)
 {
        EP_ASSERT (provider != NULL);
        EP_ASSERT (provider_callback_data != NULL);
@@ -81,7 +83,8 @@ provider_prepare_callback_data (
                provider->callback_data,
                keywords,
                provider_level,
-               provider->sessions != 0);
+               provider->sessions != 0,
+               session_id);
 }
 
 static
@@ -299,7 +302,8 @@ provider_set_config (
        int64_t keywords,
        EventPipeEventLevel level,
        const ep_char8_t *filter_data,
-       EventPipeProviderCallbackData *callback_data)
+       EventPipeProviderCallbackData *callback_data,
+       EventPipeSessionID session_id)
 {
        EP_ASSERT (provider != NULL);
        EP_ASSERT ((provider->sessions & session_mask) == 0);
@@ -311,7 +315,7 @@ provider_set_config (
        provider->provider_level = level_for_all_sessions;
 
        provider_refresh_all_events (provider);
-       provider_prepare_callback_data (provider, provider->keywords, provider->provider_level, filter_data, callback_data);
+       provider_prepare_callback_data (provider, provider->keywords, provider->provider_level, filter_data, callback_data, session_id);
 
        ep_requires_lock_held ();
        return callback_data;
@@ -340,7 +344,7 @@ provider_unset_config (
        provider->provider_level = level_for_all_sessions;
 
        provider_refresh_all_events (provider);
-       provider_prepare_callback_data (provider, provider->keywords, provider->provider_level, filter_data, callback_data);
+       provider_prepare_callback_data (provider, provider->keywords, provider->provider_level, filter_data, callback_data, (EventPipeSessionID)0);
 
        ep_requires_lock_held ();
        return callback_data;
@@ -360,6 +364,7 @@ provider_invoke_callback (EventPipeProviderCallbackData *provider_callback_data)
        int64_t keywords = ep_provider_callback_data_get_keywords (provider_callback_data);
        EventPipeEventLevel provider_level = ep_provider_callback_data_get_provider_level (provider_callback_data);
        void *callback_data = ep_provider_callback_data_get_callback_data (provider_callback_data);
+       EventPipeSessionID session_id = ep_provider_callback_data_get_session_id (provider_callback_data);
 
        bool is_event_filter_desc_init = false;
        EventFilterDescriptor event_filter_desc;
@@ -405,7 +410,7 @@ provider_invoke_callback (EventPipeProviderCallbackData *provider_callback_data)
        if (callback_function && !ep_rt_process_shutdown ()) {
                ep_rt_provider_invoke_callback (
                        callback_function,
-                       NULL, /* provider_id */
+                       (uint8_t *)&session_id, /* session_id */
                        enabled ? 1 : 0, /* ControlCode */
                        (uint8_t)provider_level,
                        (uint64_t)keywords,
index ffe7188..6f61454 100644 (file)
@@ -69,6 +69,7 @@ struct _EventPipeProviderCallbackData_Internal {
        int64_t keywords;
        EventPipeEventLevel provider_level;
        bool enabled;
+       EventPipeSessionID session_id;
 };
 
 #if !defined(EP_INLINE_GETTER_SETTER) && !defined(EP_IMPL_EP_GETTER_SETTER)
@@ -83,6 +84,7 @@ EP_DEFINE_GETTER(EventPipeProviderCallbackData *, provider_callback_data, void *
 EP_DEFINE_GETTER(EventPipeProviderCallbackData *, provider_callback_data, int64_t, keywords)
 EP_DEFINE_GETTER(EventPipeProviderCallbackData *, provider_callback_data, EventPipeEventLevel, provider_level)
 EP_DEFINE_GETTER(EventPipeProviderCallbackData *, provider_callback_data, bool, enabled)
+EP_DEFINE_GETTER(EventPipeProviderCallbackData *, provider_callback_data, EventPipeSessionID, session_id)
 
 EventPipeProviderCallbackData *
 ep_provider_callback_data_alloc (
@@ -91,7 +93,8 @@ ep_provider_callback_data_alloc (
        void *callback_data,
        int64_t keywords,
        EventPipeEventLevel provider_level,
-       bool enabled);
+       bool enabled,
+       EventPipeSessionID session_id);
 
 EventPipeProviderCallbackData *
 ep_provider_callback_data_alloc_copy (EventPipeProviderCallbackData *provider_callback_data_src);
@@ -107,7 +110,8 @@ ep_provider_callback_data_init (
        void *callback_data,
        int64_t keywords,
        EventPipeEventLevel provider_level,
-       bool enabled);
+       bool enabled,
+       EventPipeSessionID session_id);
 
 EventPipeProviderCallbackData *
 ep_provider_callback_data_init_copy (
index 617fb47..dae4281 100644 (file)
@@ -217,7 +217,8 @@ ep_provider_callback_data_alloc (
        void *callback_data,
        int64_t keywords,
        EventPipeEventLevel provider_level,
-       bool enabled)
+       bool enabled,
+       EventPipeSessionID session_id)
 {
        EventPipeProviderCallbackData *instance = ep_rt_object_alloc (EventPipeProviderCallbackData);
        ep_raise_error_if_nok (instance != NULL);
@@ -229,7 +230,8 @@ ep_provider_callback_data_alloc (
                callback_data,
                keywords,
                provider_level,
-               enabled) != NULL);
+               enabled,
+               session_id) != NULL);
 
 ep_on_exit:
        return instance;
@@ -288,7 +290,8 @@ ep_provider_callback_data_init (
        void *callback_data,
        int64_t keywords,
        EventPipeEventLevel provider_level,
-       bool enabled)
+       bool enabled,
+       EventPipeSessionID session_id)
 {
        EP_ASSERT (provider_callback_data != NULL);
 
@@ -298,6 +301,7 @@ ep_provider_callback_data_init (
        provider_callback_data->keywords = keywords;
        provider_callback_data->provider_level = provider_level;
        provider_callback_data->enabled = enabled;
+       provider_callback_data->session_id = session_id;
 
        return provider_callback_data;
 }
index 01ac979..d508f5c 100644 (file)
         <ExcludeList Include = "$(XunitTestBinBase)/tracing/eventpipe/eventsourceerror/**">
             <Issue> needs triage </Issue>
         </ExcludeList>
+        <ExcludeList Include = "$(XunitTestBinBase)/tracing/eventpipe/enabledisable/**">
+            <Issue> needs triage </Issue>
+        </ExcludeList>
         <ExcludeList Include = "$(XunitTestBinBase)/Exceptions/ForeignThread/ForeignThreadExceptions/**">
             <Issue>needs triage</Issue>
         </ExcludeList>
diff --git a/src/tests/tracing/eventpipe/enabledisable/enabledisable.cs b/src/tests/tracing/eventpipe/enabledisable/enabledisable.cs
new file mode 100644 (file)
index 0000000..80b0b54
--- /dev/null
@@ -0,0 +1,120 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Diagnostics.NETCore.Client;
+using Microsoft.Diagnostics.Tracing;
+using Microsoft.Diagnostics.Tracing.Parsers.Clr;
+using System;
+using System.Collections.Concurrent;
+using System.Diagnostics;
+using System.Diagnostics.Tracing;
+using System.IO;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using System.Collections.Generic;
+
+namespace Tracing.Tests.EnableDisableValidation
+{
+    [EventSource(Name = "Local.TestEventSource")]
+    public sealed class TestEventSource : EventSource
+    {
+        private int _disables;
+        private int _enables;
+
+        public int Enables => _enables;
+        public int Disables => _disables;
+
+        private TestEventSource()
+        {
+        }
+
+        public static TestEventSource Log = new TestEventSource();
+
+        [Event(1)]
+        public void TestEvent()
+        {
+            WriteEvent(1);
+        }
+
+        [NonEvent]
+        protected override void OnEventCommand(EventCommandEventArgs command)
+        {
+            base.OnEventCommand(command);
+
+            if (command.Command == EventCommand.Enable)
+            {
+                Interlocked.Increment(ref _enables);
+            }
+            else if (command.Command == EventCommand.Disable)
+            {
+                Interlocked.Increment(ref _disables);
+            }
+        }
+    }
+
+    public class EnableDisableValidation
+    {
+        public static int Main()
+        {
+            // There is a potential deadlock because EventPipeEventSource uses ConcurrentDictionary, which
+            // triggers loading the CDSCollectionETWBCLProvider EventSource, and registering the provider
+            // can deadlock with the writing thread. Force it to be created now.
+            ConcurrentDictionary<int, int> cd = new ConcurrentDictionary<int, int>(Environment.ProcessorCount, 0);
+            if (cd.Count > 0)
+            {
+                throw new Exception("This shouldn't ever happen");
+            }
+
+            var providers = new List<EventPipeProvider>()
+            {
+                new EventPipeProvider("Local.TestEventSource", EventLevel.Verbose)
+            };
+
+            DiagnosticsClient client = new DiagnosticsClient(Process.GetCurrentProcess().Id);
+            using (EventPipeSession session1 = client.StartEventPipeSession(providers))
+            {
+                EventPipeEventSource source1 = new EventPipeEventSource(session1.EventStream);
+
+                using (EventPipeSession session2 = client.StartEventPipeSession(providers))
+                {
+                    EventPipeEventSource source2 = new EventPipeEventSource(session2.EventStream);
+
+                    using (EventPipeSession session3 = client.StartEventPipeSession(providers))
+                    {
+                        EventPipeEventSource source3 = new EventPipeEventSource(session3.EventStream);
+                        for (int i = 0; i < 10; ++i)
+                        {
+                            TestEventSource.Log.TestEvent();
+                        }
+
+                        StopSession(session3, source3);
+                    }
+
+                    StopSession(session2, source2);
+                }
+
+                StopSession(session1, source1);
+            }
+
+            if (TestEventSource.Log.Enables > 0 &&
+                TestEventSource.Log.Enables == TestEventSource.Log.Disables)
+            {
+                return 100;
+            }
+
+            Console.WriteLine($"Test failed, enables={TestEventSource.Log.Enables} disables={TestEventSource.Log.Disables}");
+            return -1;
+        }
+
+        private static void StopSession(EventPipeSession session, EventPipeEventSource source)
+        {
+            source.Dynamic.All += (TraceEvent traceEvent) =>
+            {
+            };
+
+            Task.Run(source.Process);
+            session.Stop();
+        }
+    }
+}
diff --git a/src/tests/tracing/eventpipe/enabledisable/enabledisable.csproj b/src/tests/tracing/eventpipe/enabledisable/enabledisable.csproj
new file mode 100644 (file)
index 0000000..88ecb83
--- /dev/null
@@ -0,0 +1,16 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFrameworkIdentifier>.NETCoreApp</TargetFrameworkIdentifier>
+    <OutputType>exe</OutputType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <JitOptimizationSensitive>true</JitOptimizationSensitive>
+    <UnloadabilityIncompatible>true</UnloadabilityIncompatible>
+    <!-- GCStress timeout: https://github.com/dotnet/runtime/issues/51133 -->
+    <GCStressIncompatible Condition="'$(TargetArchitecture)' == 'arm64' and '$(TargetOS)' == 'osx'">true</GCStressIncompatible>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <ProjectReference Include="../common/common.csproj" />
+    <ProjectReference Include="../common/Microsoft.Diagnostics.NETCore.Client/Microsoft.Diagnostics.NETCore.Client.csproj" />
+  </ItemGroup>
+</Project>