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

Support inter-process named mutexes in managed Mutex implementation #48720

Open
CoffeeFlux opened this issue Feb 24, 2021 · 8 comments
Open

Support inter-process named mutexes in managed Mutex implementation #48720

CoffeeFlux opened this issue Feb 24, 2021 · 8 comments

Comments

@CoffeeFlux
Copy link
Contributor

CoffeeFlux commented Feb 24, 2021

Update in-process named mutexes are now implemented since #55199 For WebAssembly and ios/Android this is sufficient.

We may still want to implement inter-process named mutex emulation using a mechanism similar to https://github.com/dotnet/runtime/blob/main/src/coreclr/pal/src/include/pal/mutex.hpp

As discussed in #55199 (comment) the approach would be to cherrypick the support from the CoreCLR PAL into a src/libraries PAL implementation.


Original
Some tests rely on this behavior, and so now with Mono migrating implementations they are temporarily disabled.

@CoffeeFlux CoffeeFlux added this to the 6.0.0 milestone Feb 24, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 24, 2021
@CoffeeFlux CoffeeFlux removed the untriaged New issue has not been triaged by the area owner label Feb 24, 2021
@lambdageek
Copy link
Member

lambdageek commented Feb 26, 2021

Edit Ah, my mistake. This is strictly more than what Mono could do before. Thanks for clarifying Stephen.


This is just for in-process named mutexes. Not inter-process named mutexes on non-Windows.

@stephentoub
Copy link
Member

stephentoub commented Feb 26, 2021

This is just for in-process named mutexes. Not inter-process named mutexes on non-Windows.

Note that coreclr supports interprocess named mutexes on Unix. If the goal here is to be able to switch coreclr to use the managed implementation as well, that support needs to be brought to the managed implementation as well, at which point hopefully it would also work on mono.

@uweigand
Copy link
Contributor

This is now also causing problems for our experimental s390x port (features/s390x in runtimelab), because we use the Mono runtime for everything on our platform. After this change, we now see failures due to use of named Mutexes in parts of the .NET SDK itself. For example, "dotnet new console" now fails with:

System.PlatformNotSupportedException: The named version of this synchronization primitive is not supported on this platform.
   at System.Threading.Mutex.CreateMutexCore(Boolean initiallyOwned, String name, Boolean& createdNew)
   at System.Threading.Mutex..ctor(Boolean initiallyOwned, String name)
   at Microsoft.TemplateEngine.Cli.New3Command.EnsureEntryMutex(String hivePath, ITemplateEngineHost host)
   at Microsoft.TemplateEngine.Cli.New3Command.Run(String commandName, ITemplateEngineHost host, ITelemetryLogger telemetryLogger, New3Callbacks callbacks, String[] args, String hivePath)
   at Microsoft.DotNet.Tools.New.NewCommandShim.Run(String[] args)
   at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, ITelemetry telemetryClient)
   at Microsoft.DotNet.Cli.Program.Main(String[] args)

Similarly, the roslyn compile server code uses named mutexes for serialization (see e.g. src/Compilers/Shared/BuildServerConnection.cs) that now cause failures. Now I understand that the old implementation in Mono was not really complete, but at least it allowed these tasks to operate ...

What is the recommended way forward here? Is there a plan to add full support for named Mutexes to the managed implementation? Or should we try to work around this by eliminiating use of named Mutexes in the templating code and roslyn?

@CoffeeFlux
Copy link
Contributor Author

The Roslyn code looks like it was adjusted to work properly with in-process named mutexes only, so adding support for that to the managed implementation would unblock you (and allow us to re-enable a lot of the test suite). I'd like to see that happen before 6.0, but I don't have immediate plans to implement it.

I don't think adding support for in-process named mutexes would be too complicated, if that's of any interest on your end? We would certainly take the change in Mono. I'll be busy with wasm work for the next couple sprints, so I would not expect any more threading implementation work on my end for another month or two.

@uweigand
Copy link
Contributor

uweigand commented Jul 6, 2021

The Roslyn code looks like it was adjusted to work properly with in-process named mutexes only, so adding support for that to the managed implementation would unblock you (and allow us to re-enable a lot of the test suite). I'd like to see that happen before 6.0, but I don't have immediate plans to implement it.

I don't think adding support for in-process named mutexes would be too complicated, if that's of any interest on your end? We would certainly take the change in Mono. I'll be busy with wasm work for the next couple sprints, so I would not expect any more threading implementation work on my end for another month or two.

It took me a while to get to that, sorry. I've now posted a PR implementing this suggestion. Let me know if there's anything I should be doing different. Thanks!

lambdageek pushed a commit that referenced this issue Jul 8, 2021
* Partially addresses #48720

* Use a dictionary to perform the name to object lookup

* Allow multiple handles to refer to a single waitable object

* Abandon mutex only when all handles refering to it are closed

* Re-enable test cases disabled due to the above issue
@lambdageek lambdageek changed the title Support named mutexes in managed Mutex implementation Support inter-process named mutexes in managed Mutex implementation Jul 8, 2021
@lambdageek lambdageek modified the milestones: 6.0.0, 7.0.0 Jul 8, 2021
@lambdageek
Copy link
Member

Since in-process named mutex support was implemented - and is sufficient for mobile scenarios - this issue has been updated to track support for inter-process named mutex emulation. Bumping milestone to net7

@lambdageek
Copy link
Member

Revisiting this a bit.

we should be really clear on the use-case for named inter-process mutexes and on the complications from trying to reuse the CoreCLR implementation:

  1. on mobile and wasm, inter-process named mutexes don't make sense - you can't spawn new processes and if you could, you don't have a shared file system for the synchronization files that coreclr uses.
  2. on platforms where mono is the only runtime, we could probably emulate the spirit of the coreclr implementation, but not the details. (the details are in C++, not C, and depend on at least 3 other primitives from the coreclr PAL that I can see - possibly more)
  3. on platforms where mono and coreclr could both be used:
    • if the goal is backward binary compat - so a running netcore3.1 app can talk to a .net vNext mono, that seems really hard - for one thing the C++ code from mutex.hpp has a memory layout that we would have to match in C.  (things like writing a C++ struct with a bool field into shared memory seem really fragile)
    • if we have more flexibility (.NET vNext talks only to itself or future versions - whether CoreCLR or Mono), we could lock down the shared data representation more rigorously and try and share it. Although I'm not sure what that will look like because the current CoreCLR implementation writes things like pthread_mutex_t and a C++ bool type into the shared memory. Maybe we use C atomic fixed-width integer types or something.

I'm not sure if (3) is really something we're striving for. If we only want (2) or a version of (3) where mono named mutexes only interoperate with other mono processes (and not with coreclr) we could probably implement something as long as we use a slightly different naming convention that doesn't collide with coreclr.

@jkotas
Copy link
Member

jkotas commented Dec 5, 2023

on platforms where mono is the only runtime, we could probably emulate the spirit of the coreclr implementation, but not the details

The implementation of named mutexes on CoreCLR is tricky and security sensitive. I do not think we would want to maintain two parallel implementations.

If we want to do something here, we should lift the CoreCLR PAL implementation to C# and share it with Mono. Yes, it is a bunch of work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants