Fix WindowsFormsApplicationBase.IsSingleInstance

[elachlan]

Add currentUserSID to GetApplicationInstanceID for use in TryCreatePipeServer to avoid system wide blocking. Used in conjunction with PipeOptions.CurrentUserOnly.

Fixes #3715 Fixes #11232 Fixes #11365

Microsoft Reviewers: Open in CodeFlow

[elachlan]

@weltkante had this suggestion and I ran with it. I think the concerns raised by @zanchey are technically valid, but block a pragmatic fix for most use cases. It might be mitigated by us using PipeOptions.CurrentUserOnly as well.

@Olina-Zhang could your team test this? I don't have a multi-user environment I can easily utilize for testing.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 95.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 74.27371%. Comparing base (48a3d7c) to head (03230e4). Report is 614 commits behind head on main.

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #11258         +/-   ##
===================================================
- Coverage   74.34305%   74.27371%   -0.06935%     
===================================================
  Files           3012        3025         +13     
  Lines         625885      626888       +1003     
  Branches       46553       46743        +190     
===================================================
+ Hits          465302      465613        +311     
- Misses        157190      157923        +733     
+ Partials        3393        3352         -41     

Flag	Coverage Δ
Debug	74.27371% <95.00000%> (-0.06935%)	:arrow_down:
integration	17.99401% <0.00000%> (-0.34132%)	:arrow_down:
production	47.00072% <87.50000%> (+0.01172%)	:arrow_up:
test	96.99432% <100.00000%> (-0.04596%)	:arrow_down:
unit	43.97653% <87.50000%> (+0.07251%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

[Olina-Zhang]

@elachlan tested this PR using following 2 apps(one is Winforms VB app, another is Winforms C#) in .Net 9.0 SDK build on a server with multi-users. Every logged in user can launch the app, issue is fixed. Thanks! VB_SingleInstance_true.zip SingleInstanceTrue_CSharp.zip

[elachlan]

I have some concerns about using the filepath of the assembly because we are calling it using Assembly.GetCallingAssembly().

This needs to be tested with self-contained single file and AOT scenarios. As far as I know for self contained single file applications I've had to use Assembly.GetEntryAssembly() to get the correct location of where the exe is located. If Assembly.GetCallingAssembly() is used it might get the location of the assembly that is extracted to the users appdata.

I am unsure if this is still the case.

@Olina-Zhang maybe your team could test this scenario?

[Olina-Zhang]

@Olina-Zhang maybe your team could test this scenario?

Use your new updated code to test these issues again, right? Or publish app with self-contained to test?

[elachlan]

The main issue to test is #11365 using the updated code.

Then additionally try #11365 but on a single file published app.

[Olina-Zhang]

Hi @elachlan, tested the updated code, here is the result:

1. GH issue https://github.com/dotnet/winforms/issues/3715 and GH issue https://github.com/dotnet/winforms/issues/11232 - Fixed
2. For GH issue: https://github.com/dotnet/winforms/issues/11365, the scenarios customer reported about launch exe from 2 different folders with one user - Fixed, we can see 2 app instances
3. For GH issue: https://github.com/dotnet/winforms/issues/11365, copy it on a server machine in 2 different folders, then launch exe from 2 different folders with different users - Launch exe successfully with different users
4. For GH issue: https://github.com/dotnet/winforms/issues/11365, published app with self-contained, put it in 2 different folders, then launch exe from different folders with one user - app can launch from one folder, and app cannot launch from another folder, get a message that "it is already installed from a different location" --- it is expected, same as the .Net framework's behavior
5. For GH issue: https://github.com/dotnet/winforms/issues/11365, published app with self-contained, copy it on a server machine in one folder, then launch exe with different users --- first user can install and launch, second user can install but cannot launch, get following exception in EventViewer:

Application: WinFormsApp1.exe
CoreCLR Version: 9.0.24.25601
.NET Version: 9.0.0-preview.5.24256.1
Description: The process was terminated due to an unhandled exception.
Exception Info: Microsoft.VisualBasic.ApplicationServices.CantStartSingleInstanceException: This single-instance application could not connect to the original instance.
   at Microsoft.VisualBasic.ApplicationServices.WindowsFormsApplicationBase.Run(String[] commandLine)
   at WinFormsApp1.Program.Main()

[elachlan]

That last scenario with published self-contained app and two users is interesting and unexpected.

We will need to investigate further.

[elachlan]

@Olina-Zhang can you retest the published self-contained failure scenario with the latest? The event viewer will have a bit more detail hopefully.

[Olina-Zhang]

@Olina-Zhang can you retest the published self-contained failure scenario with the latest? The event viewer will have a bit more detail hopefully.

I didn't see any difference with updated code, same as Yesterday's:

[elachlan]

My change was to pass in the GetApplicationInstanceID value to the exception so we could see what it was. But it seems not to have worked.

[elachlan]

@lonitra I cannot proceed with this PR as I don't have the resources for testing. I suggest we have @LeafShi1 and team take over this work as they are better resourced. I'd also advocate for a backport to 9.0 as its blocking migrations.