Skip to content

Refine the CmdPal launch and activation logic when opening a new app instance #39377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lei9444
Copy link
Contributor

@lei9444 lei9444 commented May 12, 2025

Summary of the Pull Request

This PR address #39376

Key changes:

  1. Ensures RedirectActivationToAsync(args) is properly awaited using .GetAwaiter().GetResult() to preserve activation data.
  2. Uses DispatcherQueue.TryEnqueue to safely handle UI thread execution and avoid COMException during window activation.
  3. Addresses a race condition in Logger.InitializeLogger where multiple instances could trigger creation of unexpected log files. The root cause is that TextWriterTraceListener attempts to create a log file even if the target file is already in use, which results in a fallback file path prefixed with a GUID.

cmdpal

PR Checklist

  • Closes: #39376
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@lei9444 lei9444 requested review from zadjii-msft and moooyo May 12, 2025 14:26
@zadjii-msft zadjii-msft added the Product-Command Palette Refers to the Command Palette utility label May 13, 2025
@lei9444 lei9444 added this to the PowerToys 0.92 milestone May 16, 2025
@yeelam-gordon yeelam-gordon requested a review from Copilot May 16, 2025 08:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refines the CmdPal launch and activation logic to address activation data preservation, COM exceptions during window activation, and a race condition in logger initialization.

  • Ensures synchronous waiting on RedirectActivationToAsync to preserve activation data.
  • Moves COM wrappers initialization ahead of redirection decision.
  • Uses DispatcherQueue.TryEnqueue to safely invoke UI thread operations.
Comments suppressed due to low confidence (2)

src/modules/cmdpal/Microsoft.CmdPal.UI/Program.cs:116

  • Consider handling potential exceptions within the DispatcherQueue callback to prevent silent failures if mainWindow.HandleLaunch(args) throws.
mainWindow.DispatcherQueue.TryEnqueue(() => {

src/modules/cmdpal/Microsoft.CmdPal.UI/Program.cs:101

  • Blocking on an asynchronous call using GetAwaiter().GetResult() can potentially lead to deadlocks; please ensure this synchronous wait is safe in the current application context.
keyInstance.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult();

@@ -34,6 +34,15 @@ private static int Main(string[] args)
return 0;
}

WinRT.ComWrappersSupport.InitializeComWrappers();
Copy link
Preview

Copilot AI May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding an inline comment explaining why COM wrappers are initialized before the redirection decision to improve clarity.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-Command Palette Refers to the Command Palette utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants