Fix SharedMemory redirection queue and add telemetry events (#6127)

dabhattimsft · Darshak Bhatti · web-flow · commit a7e1ce1a7ae0 · 2026-01-29T09:56:37.000-08:00
* cp

* comments

* event name changes

* event name changes

* cleanup

* cp

* cp

---------

Co-authored-by: Darshak Bhatti &lt;dabhatti@micorsoft.com&gt;
diff --git a/dev/AppLifecycle/AppInstance.cpp b/dev/AppLifecycle/AppInstance.cpp
@@ -1,4 +1,4 @@
-﻿// Copyright (c) Microsoft Corporation and Contributors.
+// Copyright (c) Microsoft Corporation and Contributors.
 // Licensed under the MIT License.
 
 #include <pch.h>
@@ -171,23 +171,27 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
     GUID AppInstance::DequeueRedirectionRequestId()
     {
         auto releaseOnExit = m_dataMutex.acquire();
+        AppLifecycleTelemetry::DequeueRedirectionRequestId();
         auto id = m_redirectionArgs.Dequeue();
         return id;
     }
 
     void AppInstance::EnqueueRedirectionRequestId(GUID id)
     {
         auto releaseOnExit = m_dataMutex.acquire();
+        AppLifecycleTelemetry::EnqueueRedirectionRequestId(id);
         m_redirectionArgs.Enqueue(id);
     }
 
     void AppInstance::ProcessRedirectionRequests()
     {
+        auto activity{ AppLifecycleTelemetry::ProcessRedirectionRequests::Start(m_processId, m_isCurrent) };
         m_innerActivated.ResetEvent();
 
         GUID id;
         while ((id = DequeueRedirectionRequestId()) != GUID_NULL)
         {
+            activity.DequeueRedirectionRequest(id);
             wil::unique_cotaskmem_string idString;
             THROW_IF_FAILED(StringFromCLSID(id, &idString));
 
@@ -199,6 +203,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
 
             // Notify the app that the redirection request is here.
             m_activatedEvent(*this, args);
+            activity.RedrectionActivatedEvent(id);
 
             std::wstring eventName = name + c_activatedEventNameSuffix;
             wil::unique_event cleanupEvent;
@@ -207,7 +212,10 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
                 // If the event is missing, it means the waiter gave up.  Ignore the error.
                 cleanupEvent.SetEvent();
             }
+            activity.RequestCleanupEvent(id);
         }
+
+        activity.Stop();
     }
 
     IAsyncAction AppInstance::QueueRequest(AppLifecycle::AppActivationArguments args)
@@ -229,6 +237,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
 
         GUID id;
         THROW_IF_FAILED(CoCreateGuid(&id));
+        auto activity{ AppLifecycleTelemetry::QueueRequest::Start(m_processId, m_isCurrent, id) };
 
         wil::unique_cotaskmem_string idString;
         THROW_IF_FAILED(StringFromCLSID(id, &idString));
@@ -248,9 +257,11 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
 
         // Signal the activation.
         m_innerActivated.SetEvent();
+        activity.InnerActivatedEvent(id);
 
         // Wait for the other instance to open the memory mapped file before exiting and cleaning our interest in it.
         cleanupEvent.wait();
+        activity.Stop();
         co_return;
     }
 
@@ -559,11 +570,13 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
 
     event_token AppInstance::Activated(EventHandler<Microsoft::Windows::AppLifecycle::AppActivationArguments> const& handler)
     {
+        AppLifecycleTelemetry::ActivatedEventAdd(m_processId);
         return m_activatedEvent.add(handler);
     }
 
     void AppInstance::Activated(event_token const& token) noexcept
     {
+        AppLifecycleTelemetry::ActivatedEventRemove(m_processId);
         m_activatedEvent.remove(token);
     }
 
diff --git a/dev/AppLifecycle/AppInstance.h b/dev/AppLifecycle/AppInstance.h
@@ -1,4 +1,4 @@
-﻿// Copyright (c) Microsoft Corporation and Contributors.
+// Copyright (c) Microsoft Corporation and Contributors.
 // Licensed under the MIT License.
 #pragma once
 
diff --git a/dev/AppLifecycle/AppLifecycle.vcxitems b/dev/AppLifecycle/AppLifecycle.vcxitems
@@ -1,4 +1,4 @@
-﻿<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <PropertyGroup Label="Globals">
     <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
@@ -40,4 +40,4 @@
     <ClInclude Include="$(MSBuildThisFileDirectory)ValueMarshaling.h" />
     <Midl Include="$(MSBuildThisFileDirectory)AppLifecycle.idl" />
   </ItemGroup>
-</Project>
+</Project>
diff --git a/dev/AppLifecycle/AppLifecycleTelemetry.h b/dev/AppLifecycle/AppLifecycleTelemetry.h
@@ -1,4 +1,4 @@
-﻿// Copyright (c) Microsoft Corporation and Contributors.
+// Copyright (c) Microsoft Corporation and Contributors.
 // Licensed under the MIT License.
 #pragma once
 
@@ -17,4 +17,42 @@ class AppLifecycleTelemetry : public wil::TraceLoggingProvider
     DEFINE_COMPLIANT_MEASURES_EVENT(RedirectActivationToAsync, PDT_ProductAndServiceUsage);
     DEFINE_COMPLIANT_MEASURES_EVENT(ActivationRegistrationManager, PDT_ProductAndServiceUsage);
     DEFINE_COMPLIANT_MEASURES_EVENT(Restart, PDT_ProductAndServiceUsage);
+
+    DEFINE_COMPLIANT_TELEMETRY_EVENT_PARAM1(ActivatedEventAdd, PDT_ProductAndServiceUsage, uint32_t, processId);
+    DEFINE_COMPLIANT_TELEMETRY_EVENT_PARAM1(ActivatedEventRemove, PDT_ProductAndServiceUsage, uint32_t, processId);
+
+    DEFINE_COMPLIANT_MEASURES_EVENT_PARAM1(EnqueueRedirectionRequestId, PDT_ProductAndServiceUsage, GUID, requestId);
+    DEFINE_COMPLIANT_MEASURES_EVENT(DequeueRedirectionRequestId, PDT_ProductAndServiceUsage);
+
+    BEGIN_COMPLIANT_MEASURES_ACTIVITY_CLASS(ProcessRedirectionRequests, PDT_ProductAndServicePerformance);
+        DEFINE_ACTIVITY_START(uint32_t processId, bool isCurrent) noexcept try
+        {
+            TraceLoggingClassWriteStart(
+                ProcessRedirectionRequests,
+                _GENERIC_PARTB_FIELDS_ENABLED,
+                TraceLoggingUInt32(processId, "processId"),
+                TraceLoggingBool(isCurrent, "isCurrent"));
+        }
+        CATCH_LOG()
+
+        DEFINE_TAGGED_COMPLIANT_MEASURES_EVENT_PARAM1(DequeueRedirectionRequest, PDT_ProductAndServicePerformance, GUID, requestId);
+        DEFINE_TAGGED_COMPLIANT_MEASURES_EVENT_PARAM1(RedrectionActivatedEvent, PDT_ProductAndServicePerformance, GUID, requestId);
+        DEFINE_TAGGED_COMPLIANT_MEASURES_EVENT_PARAM1(RequestCleanupEvent, PDT_ProductAndServicePerformance, GUID, requestId);
+    END_ACTIVITY_CLASS();
+
+    BEGIN_COMPLIANT_MEASURES_ACTIVITY_CLASS(QueueRequest, PDT_ProductAndServicePerformance)
+        DEFINE_ACTIVITY_START(uint32_t processId, bool isCurrent, GUID requestId) noexcept try
+        {
+            TraceLoggingClassWriteStart(
+                QueueRequest,
+                _GENERIC_PARTB_FIELDS_ENABLED,
+                TraceLoggingUInt32(processId, "processId"),
+                TraceLoggingBool(isCurrent, "isCurrent"),
+                TraceLoggingGuid(requestId, "requestId"));
+        }
+        CATCH_LOG()
+
+        DEFINE_TAGGED_COMPLIANT_MEASURES_EVENT_PARAM1(InnerActivatedEvent, PDT_ProductAndServicePerformance, GUID, requestId);
+    END_ACTIVITY_CLASS()
+
 };
diff --git a/dev/AppLifecycle/RedirectionRequestQueue.h b/dev/AppLifecycle/RedirectionRequestQueue.h
@@ -1,4 +1,4 @@
-﻿// Copyright (c) Microsoft Corporation and Contributors.
+// Copyright (c) Microsoft Corporation and Contributors.
 // Licensed under the MIT License.
 #pragma once
 #include "SharedMemory.h"
@@ -7,6 +7,8 @@
 
 namespace winrt::Microsoft::Windows::AppLifecycle::implementation
 {
+    constexpr size_t c_queueSize = 4096;
+
     class RedirectionRequestQueue
     {
         struct QueueItem
@@ -16,15 +18,38 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
             GUID id{ 0 };
         };
 
+        // NOTE: SharedMemory layout
+        // In shared memory, we store headpointer (sizeof(size_t)) followed by c_queueSize QueueItems.
+        // But shared memory also stores the above requested size (head + queue) at the beginning of the memory
+        //      See SharedMemory.h:
+        //          Result of MapViewOfFile() is stored in m_view, which is of type DynamicSharedMemory<T>*.
+        //          DynamicSharedMemory has a "size" member at the beginning, followed by "data" member.
+        // m_data.Get() returns pointer to "data" member of above DynamicSharedMemory. This is our "usable" region i.e. head + queue.
+        // So the overall layout in memory can be represented as below:
+        // | DynamicSharedMemory.size |-------------------------- DynamicSharedMemory.data ---------------------------|
+        //
+        //                              m_data.Get() (start of head pointer storage)
+        //                              v
+        // | DynamicSharedMemory.size | head pointer | QueueItem[0] | QueueItem[1] | ... | QueueItem[c_queueSize - 1] |
+        //                                             ^
+        //                                             m_dataStart (first QueueItem in shared memory)
+
     public:
         void Init(const std::wstring& name)
         {
             m_name = name;
 
             // We store the head pointer at the beginning of the memory, and then items in the queue after.
-            m_data.Open(name, (sizeof(QueueItem) * 4096) + sizeof(QueueItem*));
-#pragma warning(suppress: 6305) // C6305: PREFast does not know m_data.Get() is compatible with "sizeof(size_t)".
-            m_dataStart = reinterpret_cast<QueueItem*>(m_data.Get() + sizeof(size_t));
+            auto headPointerSizeInBytes = sizeof(size_t);
+            auto queueSizeInBytes = sizeof(QueueItem) * c_queueSize;
+
+            auto totalSharedMemoryDataSizeInBytes = queueSizeInBytes + headPointerSizeInBytes;
+
+            m_data.Open(name, totalSharedMemoryDataSizeInBytes);
+
+            auto sharedMemoryBase = reinterpret_cast<std::byte*>(m_data.Get());
+            auto queueStartInSharedMemory = sharedMemoryBase + headPointerSizeInBytes;
+            m_dataStart = reinterpret_cast<QueueItem*>(queueStartInSharedMemory);
         }
 
         void Enqueue(const GUID& itemId)
@@ -120,23 +145,28 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
 
         QueueItem* AllocateItem()
         {
-            QueueItem* upperBounds = reinterpret_cast<QueueItem*>(m_data.Get()) + m_data.Size();
+            // m_dataStart points to the first QueueItem in the shared memory.
+            // The total size of the queue is c_queueSize items.
+            // m_dataStart + c_queueSize will point to one past the end of the queue.
+            // For example, if c_queueSize is 4, and m_dataStart is at address 0x22506a80010
+            // then m_dataStart + c_queueSize is at address 0x0000022506a80090.
+            // Valid items are at:
+            //    0x0000022506a80010, 0x0000022506a80030, 0x0000022506a80050, 0x0000022506a80070
+            // Since cur is of type QueueItem*, incrementing it moves by sizeof(QueueItem) in below loop.
+            QueueItem* upperBounds = m_dataStart + c_queueSize;
             auto cur = m_dataStart;
 
-#pragma warning(suppress: 6305) // C6305: PREFast does not know upperBounds was also computed in byte count so it is compatible with "sizeof(QueueItem)".
-            while (cur < (upperBounds - sizeof(QueueItem)) && cur->inUse)
+            while (cur < upperBounds && cur->inUse)
             {
-#pragma warning(suppress: 6305) // C6305: PREFast does not know cur was also computed in byte count so it is compatible with "sizeof(QueueItem)".
-                cur += sizeof(QueueItem);
+                cur++; // cur is of type QueueItem*, so ++ moves by sizeof(QueueItem).
             }
 
-#pragma warning(suppress: 6305) // C6305: PREFast does not know upperBounds was also computed in byte count so it is compatible with "sizeof(QueueItem)".
-            THROW_HR_IF(E_OUTOFMEMORY, cur >= (upperBounds - sizeof(QueueItem)));
+            THROW_HR_IF(E_OUTOFMEMORY, cur >= upperBounds);
             return cur;
         }
 
         std::wstring m_name;
-        QueueItem* m_dataStart{ nullptr };
+        QueueItem* m_dataStart{ nullptr }; // First "QueueItem" in the shared memory after the head pointer.
         SharedMemory<size_t> m_data;
     };
 }
diff --git a/dev/AppLifecycle/SharedMemory.h b/dev/AppLifecycle/SharedMemory.h
@@ -1,4 +1,4 @@
-﻿// Copyright (c) Microsoft Corporation and Contributors.
+// Copyright (c) Microsoft Corporation and Contributors.
 // Licensed under the MIT License.
 #pragma once
 
@@ -23,6 +23,8 @@ class SharedMemory
         if (createdFile)
         {
             Clear();
+
+            // We only store size of the request.
             m_view.get()->size = size;
         }
         else
@@ -31,6 +33,7 @@ class SharedMemory
             auto newSize = m_view.get()->size;
             m_view.reset();
             m_file.reset();
+
             OpenInternal(newSize);
         }
 
@@ -45,6 +48,7 @@ class SharedMemory
     void Clear()
     {
         // Clear only the data portion, not the size.
+        // and Size() accounts for only requested size. See comment in Open().
         memset(Get(), 0, Size());
     }
 
@@ -54,6 +58,7 @@ class SharedMemory
         Reset();
 
         m_name = name;
+
         OpenInternal(size);
         m_view.get()->size = size;
     }
@@ -88,11 +93,13 @@ class SharedMemory
 protected:
     bool OpenInternal(size_t size)
     {
-        m_file = wil::unique_handle(CreateFileMapping(INVALID_HANDLE_VALUE, nullptr, PAGE_READWRITE, 0, static_cast<DWORD>(size), m_name.c_str()));
+        // OpenInternal needs to account for "size" member of DynamicSharedMemory struct.
+        size_t totalSize = size + sizeof(size_t);
+        m_file = wil::unique_handle(CreateFileMapping(INVALID_HANDLE_VALUE, nullptr, PAGE_READWRITE, 0, static_cast<DWORD>(totalSize), m_name.c_str()));
         THROW_LAST_ERROR_IF_NULL(m_file);
 
         bool createdFile = (GetLastError() != ERROR_ALREADY_EXISTS);
-        m_view.reset(reinterpret_cast<DynamicSharedMemory<T>*>(MapViewOfFile(m_file.get(), FILE_MAP_READ | FILE_MAP_WRITE, 0, 0, size)));
+        m_view.reset(reinterpret_cast<DynamicSharedMemory<T>*>(MapViewOfFile(m_file.get(), FILE_MAP_READ | FILE_MAP_WRITE, 0, 0, totalSize)));
         THROW_LAST_ERROR_IF_NULL(m_view);
 
         return createdFile;

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// Copyright (c) Microsoft Corporation and Contributors.`
	`1`	`+// Copyright (c) Microsoft Corporation and Contributors.`
`2`	`2`	`// Licensed under the MIT License.`
`3`	`3`
`4`	`4`	`#include <pch.h>`
`@@ -171,23 +171,27 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`171`	`171`	`GUID AppInstance::DequeueRedirectionRequestId()`
`172`	`172`	`{`
`173`	`173`	`auto releaseOnExit = m_dataMutex.acquire();`
	`174`	`+ AppLifecycleTelemetry::DequeueRedirectionRequestId();`
`174`	`175`	`auto id = m_redirectionArgs.Dequeue();`
`175`	`176`	`return id;`
`176`	`177`	`}`
`177`	`178`
`178`	`179`	`void AppInstance::EnqueueRedirectionRequestId(GUID id)`
`179`	`180`	`{`
`180`	`181`	`auto releaseOnExit = m_dataMutex.acquire();`
	`182`	`+ AppLifecycleTelemetry::EnqueueRedirectionRequestId(id);`
`181`	`183`	`m_redirectionArgs.Enqueue(id);`
`182`	`184`	`}`
`183`	`185`
`184`	`186`	`void AppInstance::ProcessRedirectionRequests()`
`185`	`187`	`{`
	`188`	`+ auto activity{ AppLifecycleTelemetry::ProcessRedirectionRequests::Start(m_processId, m_isCurrent) };`
`186`	`189`	`m_innerActivated.ResetEvent();`
`187`	`190`
`188`	`191`	`GUID id;`
`189`	`192`	`while ((id = DequeueRedirectionRequestId()) != GUID_NULL)`
`190`	`193`	`{`
	`194`	`+ activity.DequeueRedirectionRequest(id);`
`191`	`195`	`wil::unique_cotaskmem_string idString;`
`192`	`196`	`THROW_IF_FAILED(StringFromCLSID(id, &idString));`
`193`	`197`
`@@ -199,6 +203,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`199`	`203`
`200`	`204`	`// Notify the app that the redirection request is here.`
`201`	`205`	`m_activatedEvent(*this, args);`
	`206`	`+ activity.RedrectionActivatedEvent(id);`
`202`	`207`
`203`	`208`	`std::wstring eventName = name + c_activatedEventNameSuffix;`
`204`	`209`	`wil::unique_event cleanupEvent;`
`@@ -207,7 +212,10 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`207`	`212`	`// If the event is missing, it means the waiter gave up. Ignore the error.`
`208`	`213`	`cleanupEvent.SetEvent();`
`209`	`214`	`}`
	`215`	`+ activity.RequestCleanupEvent(id);`
`210`	`216`	`}`
	`217`	`+`
	`218`	`+ activity.Stop();`
`211`	`219`	`}`
`212`	`220`
`213`	`221`	`IAsyncAction AppInstance::QueueRequest(AppLifecycle::AppActivationArguments args)`
`@@ -229,6 +237,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`229`	`237`
`230`	`238`	`GUID id;`
`231`	`239`	`THROW_IF_FAILED(CoCreateGuid(&id));`
	`240`	`+ auto activity{ AppLifecycleTelemetry::QueueRequest::Start(m_processId, m_isCurrent, id) };`
`232`	`241`
`233`	`242`	`wil::unique_cotaskmem_string idString;`
`234`	`243`	`THROW_IF_FAILED(StringFromCLSID(id, &idString));`
`@@ -248,9 +257,11 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`248`	`257`
`249`	`258`	`// Signal the activation.`
`250`	`259`	`m_innerActivated.SetEvent();`
	`260`	`+ activity.InnerActivatedEvent(id);`
`251`	`261`
`252`	`262`	`// Wait for the other instance to open the memory mapped file before exiting and cleaning our interest in it.`
`253`	`263`	`cleanupEvent.wait();`
	`264`	`+ activity.Stop();`
`254`	`265`	`co_return;`
`255`	`266`	`}`
`256`	`267`
`@@ -559,11 +570,13 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`559`	`570`
`560`	`571`	`event_token AppInstance::Activated(EventHandler<Microsoft::Windows::AppLifecycle::AppActivationArguments> const& handler)`
`561`	`572`	`{`
	`573`	`+ AppLifecycleTelemetry::ActivatedEventAdd(m_processId);`
`562`	`574`	`return m_activatedEvent.add(handler);`
`563`	`575`	`}`
`564`	`576`
`565`	`577`	`void AppInstance::Activated(event_token const& token) noexcept`
`566`	`578`	`{`
	`579`	`+ AppLifecycleTelemetry::ActivatedEventRemove(m_processId);`
`567`	`580`	`m_activatedEvent.remove(token);`
`568`	`581`	`}`
`569`	`582`