Skip to content
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

windows: Implement single instance #15371

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

JunkuiZhang
Copy link
Contributor

@JunkuiZhang JunkuiZhang commented Jul 28, 2024

This PR implements a single instance mechanism using the CreateEventW function to create a mutex. If the identifier name begins with Local, the single instance applies only to processes under the same user. If the identifier begins with Global, it applies to all users.

Additionally, I was thinking that perhaps we should integrate the single instance functionality into gpui. I believe applications developed using gpui would benefit from this feature. Furthermore, incorporating the single instance implementation into gpui would facilitate the set_dock_menu functionality. As I mentioned in #12068, the implementation of set_dock_menu on Windows depends on the single instance feature. When a user clicks the "dock menu", Windows will open a new application instance. To achieve behavior similar to macOS, we need to prevent the new instance from launching and instead pass the parameters to the existing instance.

Any advice and suggestions are welcome.

Screen.Recording.2024-08-15.234336.mp4

Release Notes:

  • N/A

@JunkuiZhang
Copy link
Contributor Author

A quick demo of dock menu on windows with changes of this PR:

Screen.Recording.2024-08-16.221846.mp4

I will open a new PR when this PR is merged.

@mikayla-maki
Copy link
Contributor

Is this PR ready to merge (after resolving conflicts)? Or is it waiting on some more development?

Copy link
Contributor

@mikayla-maki mikayla-maki left a comment

Choose a reason for hiding this comment

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

Hello!

I think we'd prefer for this to be implemented entirely in the zed crate if possible. GPUI should be laser focused on windows and rendering, and platform specific app architecture is probably outside of it's purview.

@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Aug 23, 2024

Is this PR ready to merge (after resolving conflicts)? Or is it waiting on some more development?

This PR requires discussions about its design before it can move forward.

I think we'd prefer for this to be implemented entirely in the zed crate if possible. GPUI should be laser focused on windows and rendering, and platform specific app architecture is probably outside of it's purview.

The situation here is quite complex. Since Windows and Linux handle dock menus very differently compared to macOS, and both heavily rely on the single instance feature, separating the single instance functionality from gpui is possible but could introduce complications in other areas. Say an example using Zed:

Currently, my approach to implementing the dock menu on Windows is as follows:

  1. Background: Assume an instance of Zed is already running with PID 1.
  2. New Window Action: When a user clicks "New Window" from the dock menu, Windows attempts to launch a new instance of Zed. During this launch, we add the startup parameter workspace::NewWindow, similar to running Command::new("zed.exe").arg("workspace::NewWindow"). This new instance has PID 2.
  3. Instance Detection and Communication: After launching, the PID 2 instance detects an already running instance (PID 1) using CreateEvent. To pass its startup parameter workspace::NewWindow to the PID 1 instance, it opens shared memory created by the PID 1 instance, writes the parameter into this memory, signals PID 1 using SetEvent, and then closes itself.
  4. Original Instance Handling: The PID 1 instance detects the event triggered by the other instance, opens the shared memory, reads the startup parameter, and performs the corresponding action, workspace::NewWindow in this example.

In this process, the single instance and dock menu features are tightly coupled, sharing two handles: one for notifying the original instance (Event), and one for passing the new instance's parameters (shared memory). And I think Linux may implement dock menu using a simliar approach.

Comment on lines 122 to 126
let single_instance_event = unsafe {
Owned::new(OpenEventW(
SYNCHRONIZATION_ACCESS_RIGHTS(SYNCHRONIZE.0),
false,
&HSTRING::from(retrieve_app_instance_event_identifier()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When initializing GPUI, the name provided for the single_instance_event during creation must match the name passed to CreateEventW when checking for an existing single instance. If we separate the single instance functionality from gpui, then GPUI would require additional information at startup to create this Event, for example, WindowsPlatform::new(event_name: &str, shared_memory_name: &str).

Furthermore, other apps developed using gpui that wishes to use the dock menu functionality would need to implement the single instance logic themselves. This code would likely be identical across different apps, so including the single instance implementation within gpui helps to avoid unnecessary code duplication and simplifies the process for developers.

Comment on lines 70 to 81
pub(crate) fn check_single_instance<F>(f: F) -> bool
where
F: FnOnce(bool) -> bool,
{
unsafe {
CreateEventW(
None,
false,
false,
&HSTRING::from(retrieve_app_instance_event_identifier()),
)
.expect("Unable to create instance sync event")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name (the HSTRING here) provided when calling CreateEventW here must match the name used in the OpenEventW call mentioned in the previous comment.

@JunkuiZhang
Copy link
Contributor Author

I just noticed that the discussion seems to be focused primarily on the dock menu feature. Therefore, I decided to limit the scope of this PR to the single instance feature. Once this PR is merged, we can further discuss the design and implementation in a dedicated PR for the dock menu.

@JunkuiZhang JunkuiZhang marked this pull request as ready for review August 23, 2024 13:18
@mikayla-maki
Copy link
Contributor

mikayla-maki commented Aug 29, 2024

Thanks for splitting those up, looks good!

@mikayla-maki mikayla-maki merged commit 3c53832 into zed-industries:main Aug 29, 2024
9 checks passed
@JunkuiZhang JunkuiZhang deleted the windows/single-instance branch September 3, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants