Skip to content

Commit 21ba5d2

Browse files
MuyuanMSCopilot
andcommitted
Address review: harden AppLifeMonitor shutdown loop
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bf381b4 commit 21ba5d2

1 file changed

Lines changed: 22 additions & 13 deletions

File tree

  • src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit

src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/AppLifeMonitor.cs

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System;
6+
using System.Diagnostics;
67
using System.Runtime.InteropServices;
78
using System.Threading;
89

@@ -84,9 +85,8 @@ private static extern nint CreateWindowExW(
8485
nint hInstance,
8586
nint lpParam);
8687

87-
[DllImport("user32.dll")]
88-
[return: MarshalAs(UnmanagedType.Bool)]
89-
private static extern bool GetMessageW(out MSG lpMsg, nint hWnd, uint wMsgFilterMin, uint wMsgFilterMax);
88+
[DllImport("user32.dll", SetLastError = true)]
89+
private static extern int GetMessageW(out MSG lpMsg, nint hWnd, uint wMsgFilterMin, uint wMsgFilterMax);
9090

9191
[DllImport("user32.dll")]
9292
private static extern nint DispatchMessageW(ref MSG lpmsg);
@@ -158,15 +158,15 @@ private void RunMessageLoop()
158158
// is loaded multiple times in the same session.
159159
var className = $"AppLifeMonitor_{Environment.ProcessId}";
160160
var classNamePtr = Marshal.StringToHGlobalUni(className);
161+
var hInstance = GetModuleHandleW(0);
162+
var classRegistered = false;
161163

162164
try
163165
{
164166
// Store the delegate in a field so it is not collected by the GC.
165167
_wndProcDelegate = HandleMessage;
166168
var wndProcPtr = Marshal.GetFunctionPointerForDelegate(_wndProcDelegate);
167169

168-
var hInstance = GetModuleHandleW(0);
169-
170170
var wndClass = new WNDCLASSW
171171
{
172172
lpfnWndProc = wndProcPtr,
@@ -177,11 +177,12 @@ private void RunMessageLoop()
177177
var atom = RegisterClassW(ref wndClass);
178178
if (atom == 0)
179179
{
180-
// Window class registration failed. The monitor is non-operational, but the
181-
// extension continues to run normally — just without graceful shutdown support.
180+
Trace.TraceWarning("AppLifeMonitor: RegisterClassW failed with error {0}", Marshal.GetLastWin32Error());
182181
return;
183182
}
184183

184+
classRegistered = true;
185+
185186
// Create a 0×0 invisible pop-up window.
186187
// This must NOT be a message-only window (HWND_MESSAGE parent) because
187188
// message-only windows are excluded from OS shutdown broadcasts.
@@ -201,27 +202,35 @@ private void RunMessageLoop()
201202

202203
if (_hwnd == 0)
203204
{
204-
// Window creation failed. The monitor is non-operational, but the extension
205-
// continues to run normally — just without graceful shutdown support.
206-
UnregisterClassW(classNamePtr, hInstance);
205+
Trace.TraceWarning("AppLifeMonitor: CreateWindowExW failed with error {0}", Marshal.GetLastWin32Error());
207206
return;
208207
}
209208

210209
// Signal that initialization succeeded before entering the message loop.
211210
// The constructor's WaitOne() unblocks here.
212211
_windowCreated.Set();
213212

214-
// Run the message loop until PostQuitMessage is called (WM_QUIT).
215-
while (GetMessageW(out var msg, nint.Zero, 0, 0))
213+
// Run the message loop until PostQuitMessage is called (WM_QUIT) or an error occurs.
214+
int ret;
215+
while ((ret = GetMessageW(out var msg, nint.Zero, 0, 0)) > 0)
216216
{
217217
TranslateMessage(ref msg);
218218
DispatchMessageW(ref msg);
219219
}
220220

221-
UnregisterClassW(classNamePtr, hInstance);
221+
if (ret == -1)
222+
{
223+
Trace.TraceWarning("AppLifeMonitor: GetMessageW failed with error {0}", Marshal.GetLastWin32Error());
224+
}
225+
222226
}
223227
finally
224228
{
229+
if (classRegistered)
230+
{
231+
UnregisterClassW(classNamePtr, hInstance);
232+
}
233+
225234
// Always signal so the constructor's WaitOne() unblocks even on failure paths.
226235
// Set() is idempotent, so calling it again after an early Set() in the try block is safe.
227236
// Note: _windowCreated is not disposed here because there is a theoretical window

0 commit comments

Comments
 (0)