From b964532bb66bea18acde64a6f93ccd4273fff3fa Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 31 Oct 2019 17:16:55 -0700 Subject: [PATCH 1/8] Add a Mutex creation extension method that takes an ACL --- .../Interop/Windows/Kernel32/Interop.Mutex.cs | 10 +- .../System.Threading.AccessControl.sln | 4 +- .../ref/System.Threading.AccessControl.cs | 5 + .../src/Resources/Strings.resx | 110 ++++++++++++- .../src/System.Threading.AccessControl.csproj | 22 ++- .../src/System/Threading/MutexAcl.cs | 86 ++++++++++ .../src/System/Threading/MutexAcl.net46.cs | 25 +++ ...ystem.Threading.AccessControl.Tests.csproj | 2 + .../tests/ThreadingAclExtensionsTests.cs | 152 ++++++++++++++++++ 9 files changed, 406 insertions(+), 10 deletions(-) create mode 100644 src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs create mode 100644 src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs diff --git a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs index 46ddd12f9b43..2ff81f7537a3 100644 --- a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs +++ b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -12,13 +13,16 @@ internal static partial class Kernel32 { internal const uint CREATE_MUTEX_INITIAL_OWNER = 0x1; - [DllImport(Interop.Libraries.Kernel32, EntryPoint = "OpenMutexW", SetLastError = true, CharSet = CharSet.Unicode)] + [DllImport(Libraries.Kernel32, EntryPoint = "OpenMutexW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle OpenMutex(uint desiredAccess, bool inheritHandle, string name); - [DllImport(Interop.Libraries.Kernel32, EntryPoint = "CreateMutexExW", SetLastError = true, CharSet = CharSet.Unicode)] + [DllImport(Libraries.Kernel32, EntryPoint = "CreateMutexExW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle CreateMutexEx(IntPtr lpMutexAttributes, string? name, uint flags, uint desiredAccess); - [DllImport(Interop.Libraries.Kernel32, SetLastError = true)] + [DllImport(Libraries.Kernel32, SetLastError = true)] internal static extern bool ReleaseMutex(SafeWaitHandle handle); + + [DllImport(Libraries.Kernel32, EntryPoint = "CreateMutexW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + internal static extern SafeWaitHandle CreateMutex(ref SECURITY_ATTRIBUTES lpMutexAttributes, bool bInitialOwner, string? name); } } diff --git a/src/System.Threading.AccessControl/System.Threading.AccessControl.sln b/src/System.Threading.AccessControl/System.Threading.AccessControl.sln index a1994361f8b8..bb4c50ca7495 100644 --- a/src/System.Threading.AccessControl/System.Threading.AccessControl.sln +++ b/src/System.Threading.AccessControl/System.Threading.AccessControl.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio 15 -VisualStudioVersion = 15.0.27213.1 +# Visual Studio Version 16 +VisualStudioVersion = 16.0.29411.138 MinimumVisualStudioVersion = 10.0.40219.1 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Threading.AccessControl.Tests", "tests\System.Threading.AccessControl.Tests.csproj", "{458E445C-DF3C-4E4D-8E1D-F2FAC365BB40}" ProjectSection(ProjectDependencies) = postProject diff --git a/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs b/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs index 8d7efcd0bf12..e843a297264d 100644 --- a/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs +++ b/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs @@ -147,4 +147,9 @@ public static void SetAccessControl(this System.Threading.EventWaitHandle handle public static void SetAccessControl(this System.Threading.Mutex mutex, System.Security.AccessControl.MutexSecurity mutexSecurity) { } public static void SetAccessControl(this System.Threading.Semaphore semaphore, System.Security.AccessControl.SemaphoreSecurity semaphoreSecurity) { } } + + public static class MutexAcl + { + public static System.Threading.Mutex Create(bool initiallyOwned, string name, out bool createdNew, System.Security.AccessControl.MutexSecurity mutexSecurity) { throw null; } + } } diff --git a/src/System.Threading.AccessControl/src/Resources/Strings.resx b/src/System.Threading.AccessControl/src/Resources/Strings.resx index 265db70986ef..2e0669e8eca8 100644 --- a/src/System.Threading.AccessControl/src/Resources/Strings.resx +++ b/src/System.Threading.AccessControl/src/Resources/Strings.resx @@ -1,4 +1,64 @@ - + + + @@ -63,4 +123,52 @@ Access Control List (ACL) APIs are part of resource management on Windows and are not supported on this platform. + + The length of the name exceeds the maximum limit. + + + Enum value was out of legal range. + + + Cannot create '{0}' because a file or directory with the same name already exists. + + + The file '{0}' already exists. + + + Unable to find the specified file. + + + Could not find file '{0}'. + + + Could not find a part of the path. + + + Could not find a part of the path '{0}'. + + + The specified file name or path is too long, or a component of the specified path is too long. + + + The path '{0}' is too long, or a component of the specified path is too long. + + + The process cannot access the file '{0}' because it is being used by another process. + + + The process cannot access the file because it is being used by another process. + + + Access to the path is denied. + + + Access to the path '{0}' is denied. + + + Argument cannot be null or empty. + + + A WaitHandle with system-wide name '{0}' cannot be created. A WaitHandle of a different type might have the same name. + \ No newline at end of file diff --git a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj index 1485cdf4b117..9c256f8d8ee4 100644 --- a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj +++ b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj @@ -1,14 +1,23 @@ - + Windows_NT SR.PlatformNotSupported_AccessControl true net461-Windows_NT-Debug;net461-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release;netstandard2.0-Debug;netstandard2.0-Release;netstandard2.0-Windows_NT-Debug;netstandard2.0-Windows_NT-Release + true + + + - - Common\Interop\Windows\Interop.Errors.cs - + + + + + + + + @@ -17,10 +26,15 @@ + + + + + \ No newline at end of file diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs new file mode 100644 index 000000000000..1d66e1d63b55 --- /dev/null +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs @@ -0,0 +1,86 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.IO; +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using Microsoft.Win32.SafeHandles; + +namespace System.Threading +{ + public static class MutexAcl + { + /// Creates a new instance, indicating whether the calling thread should have initial ownership of the mutex, the name of the system mutex, a boolean that, when the method returns, indicates whether the calling thread was granted initial ownership of the mutex, allowing specifying the mutex security. + /// to give the calling thread initial ownership of the named system mutex if the named system mutex is created as a result of this call; otherwise, . + /// The name of the system mutex. + /// When this method returns, it is set to if a local mutex was created (that is, if name is or an empty string) or if the specified named system mutex was created; if the specified named system mutex already existed. This parameter is passed uninitialized. + /// An object that represents the access control security to be applied to the named system mutex. + /// An object that represents the named system mutex. + /// Argument cannot be null or empty. + /// -or- + /// The length of the name exceeds the maximum limit. + /// + /// is . + /// A mutex handle with the system-wide cannot be created. A mutex handle of a different type might have the same name. + public static unsafe Mutex Create(bool initiallyOwned, string name, out bool createdNew, MutexSecurity mutexSecurity) + { + if (string.IsNullOrEmpty(name)) + { + throw new ArgumentException(SR.Format(SR.Argument_CannotBeNullOrEmpty), nameof(name)); + } + if (mutexSecurity == null) + { + throw new ArgumentNullException(nameof(mutexSecurity)); + } + + fixed (byte* pSecurityDescriptor = mutexSecurity.GetSecurityDescriptorBinaryForm()) + { + var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES + { + nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES), + lpSecurityDescriptor = (IntPtr)pSecurityDescriptor + }; + + using SafeWaitHandle mutexHandle = Interop.Kernel32.CreateMutex(ref secAttrs, initiallyOwned, name); + + ValidateMutexHandle(mutexHandle, name, out createdNew); + + try + { + return Mutex.OpenExisting(name); + } + finally + { + // Close our handle as the Mutex will have it's own. + mutexHandle.Dispose(); + } + } + } + + private static void ValidateMutexHandle(SafeWaitHandle mutexHandle, string name, out bool createdNew) + { + int errorCode = Marshal.GetLastWin32Error(); + + if (mutexHandle.IsInvalid) + { + mutexHandle.SetHandleAsInvalid(); + + if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) + { + // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 + throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); + } + + if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) + { + throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); + } + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); + } + + createdNew = (errorCode != Interop.Errors.ERROR_ALREADY_EXISTS); + } + } +} diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs new file mode 100644 index 000000000000..912d2f4497c6 --- /dev/null +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Security.AccessControl; + +namespace System.Threading +{ + public static class MutexAcl + { + public static Mutex Create(bool initiallyOwned, string name, out bool createdNew, MutexSecurity mutexSecurity) + { + if (string.IsNullOrEmpty(name)) + { + throw new ArgumentException(SR.Format(SR.Argument_CannotBeNullOrEmpty), nameof(name)); + } + if (mutexSecurity == null) + { + throw new ArgumentNullException(nameof(mutexSecurity)); + } + + return new Mutex(initiallyOwned, name, createdNew, mutexSecurity); + } + } +} diff --git a/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj b/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj index 62b99a1f8291..223103bf6c9e 100644 --- a/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj +++ b/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj @@ -3,6 +3,8 @@ netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release + + diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 066b246fcd71..3bbbc4a95b94 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -2,13 +2,27 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; +using System.Linq; using System.Security.AccessControl; +using System.Security.Principal; using Xunit; namespace System.Threading.Tests { public static class ThreadingAclExtensionsTests { + // Dec: 34603010 + // Hex: 0x2100002 + // As predefined by the different wait handles: + // - https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/EventWaitHandle.Windows.cs + // - https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/Mutex.Windows.cs + private const int BasicAccessRights = Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE; + + #region Test methods + + #region Existence tests + [Fact] [PlatformSpecific(TestPlatforms.Windows)] // APIs not supported on Unix public static void ExistenceTest_Windows() @@ -45,5 +59,143 @@ public static void ExistenceTest_Unix() Assert.Throws(() => m.GetAccessControl()); Assert.Throws(() => m.SetAccessControl(new MutexSecurity())); } + + #endregion + + #region Mutex + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void Mutex_Create_NullSecurity() + { + AssertExtensions.Throws("eventSecurity", () => + { + using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, "Test", out bool createdNew, mutexSecurity: null); + }); + } + + [Theory] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(null)] + [InlineData("")] + public static void Mutex_Create_InvalidName(string name) + { + AssertExtensions.Throws("name", () => + { + using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, name, out bool createdNew, GetBasicMutexSecurity()); + }); + } + + [Fact] + public static void Mutex_Create_BeyondMaxLengthName() + { + AssertExtensions.Throws("name", () => + { + string name = new string('X', Interop.Kernel32.MAX_PATH + 1); + using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, name, out bool createdNew, GetBasicMutexSecurity()); + }); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void Mutex_Create_BasicSecurity() + { + var security = GetBasicMutexSecurity(); + CreateAndVerifyMutex(security); + } + + [Theory] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true, AccessControlType.Allow, BasicAccessRights)] + [InlineData(false, AccessControlType.Allow, BasicAccessRights)] + [InlineData(true, AccessControlType.Deny, BasicAccessRights)] + [InlineData(false, AccessControlType.Deny, BasicAccessRights)] + + public static void Mutex_Create_SpecificParameters(bool initiallyOwned, AccessControlType accessControl, int rights) + { + var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, (MutexRights)rights, accessControl); + CreateAndVerifyMutex(initiallyOwned, security); + + } + + #endregion + + #endregion + + #region Helper methods + + private static string GetRandomNameMaxLength() + { + return Guid.NewGuid().ToString("N"); + } + + private static MutexSecurity GetBasicMutexSecurity() + { + return GetMutexSecurity( + WellKnownSidType.BuiltinUsersSid, + (MutexRights)BasicAccessRights, + AccessControlType.Allow); + } + + private static MutexSecurity GetMutexSecurity(WellKnownSidType sid, MutexRights rights, AccessControlType accessControl) + { + var security = new MutexSecurity(); + SecurityIdentifier identity = new SecurityIdentifier(sid, null); + var accessRule = new MutexAccessRule(identity, rights, accessControl); + security.AddAccessRule(accessRule); + return security; + } + + private static void CreateAndVerifyMutex(MutexSecurity security) + { + CreateAndVerifyMutex(initiallyOwned: true, security); + } + + private static void CreateAndVerifyMutex(bool initiallyOwned, MutexSecurity expectedSecurity) + { + string name = GetRandomNameMaxLength(); + + using Mutex eventHandle = MutexAcl.Create(initiallyOwned, name, out bool createdNew, expectedSecurity); + + Assert.NotNull(eventHandle); + Assert.True(createdNew); + + MutexSecurity actualSecurity = eventHandle.GetAccessControl(); + + VerifyMutexSecurity(expectedSecurity, actualSecurity); + } + + private static void VerifyMutexSecurity(MutexSecurity expectedSecurity, MutexSecurity actualSecurity) + { + Assert.Equal(typeof(MutexRights), expectedSecurity.AccessRightType); + Assert.Equal(typeof(MutexRights), actualSecurity.AccessRightType); + + List expectedAccessRules = expectedSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) + .Cast().ToList(); + + List actualAccessRules = actualSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) + .Cast().ToList(); + + Assert.Equal(expectedAccessRules.Count, actualAccessRules.Count); + if (expectedAccessRules.Count > 0) + { + Assert.All(expectedAccessRules, actualAccessRule => + { + int count = expectedAccessRules.Count(expectedAccessRule => AreMutexAccessRulesEqual(expectedAccessRule, actualAccessRule)); + Assert.True(count > 0); + }); + } + } + + private static bool AreMutexAccessRulesEqual(MutexAccessRule expectedRule, MutexAccessRule actualRule) + { + return + expectedRule.AccessControlType == actualRule.AccessControlType && + expectedRule.MutexRights == actualRule.MutexRights && + expectedRule.InheritanceFlags == actualRule.InheritanceFlags && + expectedRule.PropagationFlags == actualRule.PropagationFlags; + } + + #endregion } } From b1297d2faf846d975f718abdb2d1217eef1fd511 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 31 Oct 2019 17:54:40 -0700 Subject: [PATCH 2/8] Fix build failures, use MutexRights in tests --- .../src/System.Threading.AccessControl.csproj | 6 ++--- .../src/System/Threading/MutexAcl.cs | 4 ++++ .../src/System/Threading/MutexAcl.net46.cs | 6 ++++- .../tests/ThreadingAclExtensionsTests.cs | 23 ++++++++----------- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj index 9c256f8d8ee4..f4792d905474 100644 --- a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj +++ b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj @@ -21,12 +21,13 @@ + - + @@ -34,7 +35,4 @@ - - - \ No newline at end of file diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs index 1d66e1d63b55..0e56b294d524 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs @@ -29,6 +29,10 @@ public static unsafe Mutex Create(bool initiallyOwned, string name, out bool cre { throw new ArgumentException(SR.Format(SR.Argument_CannotBeNullOrEmpty), nameof(name)); } + if (name.Length > Interop.Kernel32.MAX_PATH) + { + throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, name), nameof(name)); + } if (mutexSecurity == null) { throw new ArgumentNullException(nameof(mutexSecurity)); diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs index 912d2f4497c6..c49c602193f4 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs @@ -14,12 +14,16 @@ public static Mutex Create(bool initiallyOwned, string name, out bool createdNew { throw new ArgumentException(SR.Format(SR.Argument_CannotBeNullOrEmpty), nameof(name)); } + if (name.Length > Interop.Kernel32.MAX_PATH) + { + throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, name), nameof(name)); + } if (mutexSecurity == null) { throw new ArgumentNullException(nameof(mutexSecurity)); } - return new Mutex(initiallyOwned, name, createdNew, mutexSecurity); + return new Mutex(initiallyOwned, name, out createdNew, mutexSecurity); } } } diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 3bbbc4a95b94..14072cfbf244 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -12,12 +12,7 @@ namespace System.Threading.Tests { public static class ThreadingAclExtensionsTests { - // Dec: 34603010 - // Hex: 0x2100002 - // As predefined by the different wait handles: - // - https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/EventWaitHandle.Windows.cs - // - https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/Mutex.Windows.cs - private const int BasicAccessRights = Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE; + private const MutexRights BasicMutexRights = MutexRights.FullControl | MutexRights.Synchronize | MutexRights.Modify; #region Test methods @@ -68,7 +63,7 @@ public static void ExistenceTest_Unix() [PlatformSpecific(TestPlatforms.Windows)] public static void Mutex_Create_NullSecurity() { - AssertExtensions.Throws("eventSecurity", () => + AssertExtensions.Throws("mutexSecurity", () => { using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, "Test", out bool createdNew, mutexSecurity: null); }); @@ -106,14 +101,14 @@ public static void Mutex_Create_BasicSecurity() [Theory] [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, AccessControlType.Allow, BasicAccessRights)] - [InlineData(false, AccessControlType.Allow, BasicAccessRights)] - [InlineData(true, AccessControlType.Deny, BasicAccessRights)] - [InlineData(false, AccessControlType.Deny, BasicAccessRights)] + [InlineData(true, AccessControlType.Allow, BasicMutexRights)] + [InlineData(false, AccessControlType.Allow, BasicMutexRights)] + [InlineData(true, AccessControlType.Deny, BasicMutexRights)] + [InlineData(false, AccessControlType.Deny, BasicMutexRights)] - public static void Mutex_Create_SpecificParameters(bool initiallyOwned, AccessControlType accessControl, int rights) + public static void Mutex_Create_SpecificParameters(bool initiallyOwned, AccessControlType accessControl, MutexRights rights) { - var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, (MutexRights)rights, accessControl); + var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); CreateAndVerifyMutex(initiallyOwned, security); } @@ -133,7 +128,7 @@ private static MutexSecurity GetBasicMutexSecurity() { return GetMutexSecurity( WellKnownSidType.BuiltinUsersSid, - (MutexRights)BasicAccessRights, + BasicMutexRights, AccessControlType.Allow); } From 39e6ded5078c293cdeb5683dfc5e4bd256151927 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 1 Nov 2019 13:35:00 -0700 Subject: [PATCH 3/8] Unit test fixing --- .../tests/ThreadingAclExtensionsTests.cs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 14072cfbf244..08b4c91c6bb9 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -12,8 +12,6 @@ namespace System.Threading.Tests { public static class ThreadingAclExtensionsTests { - private const MutexRights BasicMutexRights = MutexRights.FullControl | MutexRights.Synchronize | MutexRights.Modify; - #region Test methods #region Existence tests @@ -65,7 +63,7 @@ public static void Mutex_Create_NullSecurity() { AssertExtensions.Throws("mutexSecurity", () => { - using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, "Test", out bool createdNew, mutexSecurity: null); + using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, GetRandomName(), out bool createdNew, mutexSecurity: null); }); } @@ -101,16 +99,18 @@ public static void Mutex_Create_BasicSecurity() [Theory] [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, AccessControlType.Allow, BasicMutexRights)] - [InlineData(false, AccessControlType.Allow, BasicMutexRights)] - [InlineData(true, AccessControlType.Deny, BasicMutexRights)] - [InlineData(false, AccessControlType.Deny, BasicMutexRights)] - + [InlineData(true, AccessControlType.Allow, MutexRights.FullControl)] + [InlineData(true, AccessControlType.Allow, MutexRights.Modify)] + [InlineData(true, AccessControlType.Deny, MutexRights.Synchronize)] + [InlineData(true, AccessControlType.Deny, MutexRights.Modify | MutexRights.Synchronize)] + [InlineData(false, AccessControlType.Allow, MutexRights.FullControl)] + [InlineData(false, AccessControlType.Allow, MutexRights.Modify)] + [InlineData(false, AccessControlType.Deny, MutexRights.Synchronize)] + [InlineData(false, AccessControlType.Deny, MutexRights.Modify | MutexRights.Synchronize)] public static void Mutex_Create_SpecificParameters(bool initiallyOwned, AccessControlType accessControl, MutexRights rights) { var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); CreateAndVerifyMutex(initiallyOwned, security); - } #endregion @@ -119,7 +119,7 @@ public static void Mutex_Create_SpecificParameters(bool initiallyOwned, AccessCo #region Helper methods - private static string GetRandomNameMaxLength() + private static string GetRandomName() { return Guid.NewGuid().ToString("N"); } @@ -128,7 +128,7 @@ private static MutexSecurity GetBasicMutexSecurity() { return GetMutexSecurity( WellKnownSidType.BuiltinUsersSid, - BasicMutexRights, + MutexRights.FullControl, AccessControlType.Allow); } @@ -148,7 +148,7 @@ private static void CreateAndVerifyMutex(MutexSecurity security) private static void CreateAndVerifyMutex(bool initiallyOwned, MutexSecurity expectedSecurity) { - string name = GetRandomNameMaxLength(); + string name = GetRandomName(); using Mutex eventHandle = MutexAcl.Create(initiallyOwned, name, out bool createdNew, expectedSecurity); @@ -186,9 +186,9 @@ private static bool AreMutexAccessRulesEqual(MutexAccessRule expectedRule, Mutex { return expectedRule.AccessControlType == actualRule.AccessControlType && - expectedRule.MutexRights == actualRule.MutexRights && - expectedRule.InheritanceFlags == actualRule.InheritanceFlags && - expectedRule.PropagationFlags == actualRule.PropagationFlags; + expectedRule.MutexRights == actualRule.MutexRights && + expectedRule.InheritanceFlags == actualRule.InheritanceFlags && + expectedRule.PropagationFlags == actualRule.PropagationFlags; } #endregion From f51644b3bf5887fdb6c47571782f03938316d0ec Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 4 Nov 2019 15:58:01 -0800 Subject: [PATCH 4/8] Use existing PInvoke, swap handle like in EventWaitHandle case, write new tests. --- .../Interop/Windows/Kernel32/Interop.Mutex.cs | 3 - .../src/System/Threading/MutexAcl.cs | 54 +++---- .../tests/ThreadingAclExtensionsTests.cs | 135 ++++++++++++------ 3 files changed, 114 insertions(+), 78 deletions(-) diff --git a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs index 2ff81f7537a3..6adaf9873704 100644 --- a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs +++ b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.Mutex.cs @@ -21,8 +21,5 @@ internal static partial class Kernel32 [DllImport(Libraries.Kernel32, SetLastError = true)] internal static extern bool ReleaseMutex(SafeWaitHandle handle); - - [DllImport(Libraries.Kernel32, EntryPoint = "CreateMutexW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern SafeWaitHandle CreateMutex(ref SECURITY_ATTRIBUTES lpMutexAttributes, bool bInitialOwner, string? name); } } diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs index 0e56b294d524..597ba1761c7b 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs @@ -11,33 +11,23 @@ namespace System.Threading { public static class MutexAcl { - /// Creates a new instance, indicating whether the calling thread should have initial ownership of the mutex, the name of the system mutex, a boolean that, when the method returns, indicates whether the calling thread was granted initial ownership of the mutex, allowing specifying the mutex security. + /// Gets or creates instance, allowing a to be optionally specified to set it during the mutex creation. /// to give the calling thread initial ownership of the named system mutex if the named system mutex is created as a result of this call; otherwise, . - /// The name of the system mutex. - /// When this method returns, it is set to if a local mutex was created (that is, if name is or an empty string) or if the specified named system mutex was created; if the specified named system mutex already existed. This parameter is passed uninitialized. - /// An object that represents the access control security to be applied to the named system mutex. - /// An object that represents the named system mutex. - /// Argument cannot be null or empty. - /// -or- - /// The length of the name exceeds the maximum limit. - /// - /// is . - /// A mutex handle with the system-wide cannot be created. A mutex handle of a different type might have the same name. + /// The optional name of the system mutex. If this argument is set to or , a local mutex is created. + /// When this method returns, this argument is always set to if a local mutex is created; that is, when is or . If has a valid non-empty value, this argument is set to when the system mutex is created, or it is set to if an existing system mutex is found with that name. This parameter is passed uninitialized. + /// The optional mutex access control security to apply. + /// An object that represents a system mutex. + /// .NET Framework only: The length of the name exceeds the maximum limit. + /// A mutex handle with system-wide cannot be created. A mutex handle of a different type might have the same name. public static unsafe Mutex Create(bool initiallyOwned, string name, out bool createdNew, MutexSecurity mutexSecurity) { - if (string.IsNullOrEmpty(name)) - { - throw new ArgumentException(SR.Format(SR.Argument_CannotBeNullOrEmpty), nameof(name)); - } - if (name.Length > Interop.Kernel32.MAX_PATH) - { - throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, name), nameof(name)); - } if (mutexSecurity == null) { - throw new ArgumentNullException(nameof(mutexSecurity)); + return new Mutex(initiallyOwned, name, out createdNew); } + uint mutexFlags = initiallyOwned ? Interop.Kernel32.CREATE_MUTEX_INITIAL_OWNER : 0; + fixed (byte* pSecurityDescriptor = mutexSecurity.GetSecurityDescriptorBinaryForm()) { var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES @@ -46,19 +36,21 @@ public static unsafe Mutex Create(bool initiallyOwned, string name, out bool cre lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - using SafeWaitHandle mutexHandle = Interop.Kernel32.CreateMutex(ref secAttrs, initiallyOwned, name); + SafeWaitHandle handle = Interop.Kernel32.CreateMutexEx( + (IntPtr)(&secAttrs), + name, + mutexFlags, + (uint)MutexRights.FullControl // Equivalent to MUTEX_ALL_ACCESS + ); - ValidateMutexHandle(mutexHandle, name, out createdNew); + ValidateMutexHandle(handle, name, out createdNew); - try - { - return Mutex.OpenExisting(name); - } - finally - { - // Close our handle as the Mutex will have it's own. - mutexHandle.Dispose(); - } + var mutex = new Mutex(initiallyOwned); + var old = mutex.SafeWaitHandle; + mutex.SafeWaitHandle = handle; + old.Dispose(); + + return mutex; } } diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 08b4c91c6bb9..5b341e5450c5 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -61,32 +61,75 @@ public static void ExistenceTest_Unix() [PlatformSpecific(TestPlatforms.Windows)] public static void Mutex_Create_NullSecurity() { - AssertExtensions.Throws("mutexSecurity", () => - { - using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, GetRandomName(), out bool createdNew, mutexSecurity: null); - }); + using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), expectedSecurity: null, expectedCreatedNew: true); } - [Theory] + [Fact] [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(null)] - [InlineData("")] - public static void Mutex_Create_InvalidName(string name) + public static void Mutex_Create_NullName() { - AssertExtensions.Throws("name", () => - { - using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, name, out bool createdNew, GetBasicMutexSecurity()); - }); + using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, name: null, GetBasicMutexSecurity(), expectedCreatedNew: true); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void Mutex_Create_NullNameMultipleNew() + { + string name = string.Empty; + var security = GetBasicMutexSecurity(); + + using Mutex mutex1 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + using Mutex mutex2 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void Mutex_Create_EmptyName() + { + using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, string.Empty, GetBasicMutexSecurity(), expectedCreatedNew: true); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void Mutex_Create_EmptyNameMultipleNew() + { + string name = string.Empty; + var security = GetBasicMutexSecurity(); + + using Mutex mutex1 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + using Mutex mutex2 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void Mutex_Create_CreateNewExisting() + { + string name = GetRandomName(); + var security = GetBasicMutexSecurity(); + + using Mutex mutexNew = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + using Mutex mutexExisting = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: false); } [Fact] - public static void Mutex_Create_BeyondMaxLengthName() + [PlatformSpecific(TestPlatforms.Windows)] + public static void Mutex_Create_BeyondMaxPathLength() { - AssertExtensions.Throws("name", () => + string name = new string('x', Interop.Kernel32.MAX_PATH + 100); + + if (PlatformDetection.IsFullFramework) + { + Assert.Throws(() => + { + using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); + }); + } + else { - string name = new string('X', Interop.Kernel32.MAX_PATH + 1); - using Mutex eventHandle = MutexAcl.Create(initiallyOwned: true, name, out bool createdNew, GetBasicMutexSecurity()); - }); + using Mutex mutex = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); + using Mutex openedByName = Mutex.OpenExisting(name); + Assert.NotNull(openedByName); + } } [Fact] @@ -94,23 +137,30 @@ public static void Mutex_Create_BeyondMaxLengthName() public static void Mutex_Create_BasicSecurity() { var security = GetBasicMutexSecurity(); - CreateAndVerifyMutex(security); + using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), security, expectedCreatedNew: true); } [Theory] [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, AccessControlType.Allow, MutexRights.FullControl)] - [InlineData(true, AccessControlType.Allow, MutexRights.Modify)] - [InlineData(true, AccessControlType.Deny, MutexRights.Synchronize)] - [InlineData(true, AccessControlType.Deny, MutexRights.Modify | MutexRights.Synchronize)] - [InlineData(false, AccessControlType.Allow, MutexRights.FullControl)] - [InlineData(false, AccessControlType.Allow, MutexRights.Modify)] - [InlineData(false, AccessControlType.Deny, MutexRights.Synchronize)] - [InlineData(false, AccessControlType.Deny, MutexRights.Modify | MutexRights.Synchronize)] - public static void Mutex_Create_SpecificParameters(bool initiallyOwned, AccessControlType accessControl, MutexRights rights) + [InlineData(true, MutexRights.FullControl, AccessControlType.Allow)] + [InlineData(true, MutexRights.FullControl, AccessControlType.Deny)] + [InlineData(true, MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, MutexRights.Modify, AccessControlType.Allow)] + [InlineData(true, MutexRights.Modify, AccessControlType.Deny)] + [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, MutexRights.FullControl, AccessControlType.Allow)] + [InlineData(false, MutexRights.FullControl, AccessControlType.Deny)] + [InlineData(false, MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(false, MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, MutexRights.Modify, AccessControlType.Allow)] + [InlineData(false, MutexRights.Modify, AccessControlType.Deny)] + public static void Mutex_Create_SpecificParameters(bool initiallyOwned, MutexRights rights, AccessControlType accessControl) { var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); - CreateAndVerifyMutex(initiallyOwned, security); + CreateAndVerifyMutex(initiallyOwned, GetRandomName(), security, expectedCreatedNew: true); + } #endregion @@ -141,23 +191,19 @@ private static MutexSecurity GetMutexSecurity(WellKnownSidType sid, MutexRights return security; } - private static void CreateAndVerifyMutex(MutexSecurity security) - { - CreateAndVerifyMutex(initiallyOwned: true, security); - } - - private static void CreateAndVerifyMutex(bool initiallyOwned, MutexSecurity expectedSecurity) + private static Mutex CreateAndVerifyMutex(bool initiallyOwned, string name, MutexSecurity expectedSecurity, bool expectedCreatedNew) { - string name = GetRandomName(); - - using Mutex eventHandle = MutexAcl.Create(initiallyOwned, name, out bool createdNew, expectedSecurity); - - Assert.NotNull(eventHandle); - Assert.True(createdNew); + Mutex mutex = MutexAcl.Create(initiallyOwned, name, out bool createdNew, expectedSecurity); + Assert.NotNull(mutex); + Assert.Equal(createdNew, expectedCreatedNew); - MutexSecurity actualSecurity = eventHandle.GetAccessControl(); + if (expectedSecurity != null) + { + MutexSecurity actualSecurity = mutex.GetAccessControl(); + VerifyMutexSecurity(expectedSecurity, actualSecurity); + } - VerifyMutexSecurity(expectedSecurity, actualSecurity); + return mutex; } private static void VerifyMutexSecurity(MutexSecurity expectedSecurity, MutexSecurity actualSecurity) @@ -176,13 +222,13 @@ private static void VerifyMutexSecurity(MutexSecurity expectedSecurity, MutexSec { Assert.All(expectedAccessRules, actualAccessRule => { - int count = expectedAccessRules.Count(expectedAccessRule => AreMutexAccessRulesEqual(expectedAccessRule, actualAccessRule)); + int count = expectedAccessRules.Count(expectedAccessRule => AreAccessRulesEqual(expectedAccessRule, actualAccessRule)); Assert.True(count > 0); }); } } - private static bool AreMutexAccessRulesEqual(MutexAccessRule expectedRule, MutexAccessRule actualRule) + private static bool AreAccessRulesEqual(MutexAccessRule expectedRule, MutexAccessRule actualRule) { return expectedRule.AccessControlType == actualRule.AccessControlType && @@ -191,6 +237,7 @@ private static bool AreMutexAccessRulesEqual(MutexAccessRule expectedRule, Mutex expectedRule.PropagationFlags == actualRule.PropagationFlags; } + #endregion } } From bd7229ba41b6f45c3926a33a78dfca6c6d570486 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 4 Nov 2019 16:01:46 -0800 Subject: [PATCH 5/8] Remove exception checks in netfx version --- .../src/System/Threading/MutexAcl.net46.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs index c49c602193f4..2c1a5caf8279 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.net46.cs @@ -10,19 +10,6 @@ public static class MutexAcl { public static Mutex Create(bool initiallyOwned, string name, out bool createdNew, MutexSecurity mutexSecurity) { - if (string.IsNullOrEmpty(name)) - { - throw new ArgumentException(SR.Format(SR.Argument_CannotBeNullOrEmpty), nameof(name)); - } - if (name.Length > Interop.Kernel32.MAX_PATH) - { - throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, name), nameof(name)); - } - if (mutexSecurity == null) - { - throw new ArgumentNullException(nameof(mutexSecurity)); - } - return new Mutex(initiallyOwned, name, out createdNew, mutexSecurity); } } From f363dadc51cd6195231a17f985dafab119152bbe Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez Date: Mon, 4 Nov 2019 23:24:53 -0800 Subject: [PATCH 6/8] Move tests to separate file --- .../tests/AclTests.cs | 10 + .../tests/MutexAclTests.cs | 148 +++++++++++++ ...ystem.Threading.AccessControl.Tests.csproj | 2 + .../tests/ThreadingAclExtensionsTests.cs | 194 ------------------ 4 files changed, 160 insertions(+), 194 deletions(-) create mode 100644 src/System.Threading.AccessControl/tests/AclTests.cs create mode 100644 src/System.Threading.AccessControl/tests/MutexAclTests.cs diff --git a/src/System.Threading.AccessControl/tests/AclTests.cs b/src/System.Threading.AccessControl/tests/AclTests.cs new file mode 100644 index 000000000000..40f9ae11aa22 --- /dev/null +++ b/src/System.Threading.AccessControl/tests/AclTests.cs @@ -0,0 +1,10 @@ +namespace System.Threading.Tests +{ + public class AclTests + { + protected string GetRandomName() + { + return Guid.NewGuid().ToString("N"); + } + } +} diff --git a/src/System.Threading.AccessControl/tests/MutexAclTests.cs b/src/System.Threading.AccessControl/tests/MutexAclTests.cs new file mode 100644 index 000000000000..14a1185aaff2 --- /dev/null +++ b/src/System.Threading.AccessControl/tests/MutexAclTests.cs @@ -0,0 +1,148 @@ +using System.Collections.Generic; +using System.Linq; +using System.Security.AccessControl; +using System.Security.Principal; +using Xunit; + +namespace System.Threading.Tests +{ + public class MutexAclTests : AclTests + { + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void Mutex_Create_NullSecurity() + { + using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), expectedSecurity: null, expectedCreatedNew: true); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [PlatformSpecific(TestPlatforms.Windows)] + public void Mutex_Create_NameMultipleNew(string name) + { + var security = GetBasicMutexSecurity(); + + using Mutex mutex1 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + using Mutex mutex2 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void Mutex_Create_CreateNewExisting() + { + string name = GetRandomName(); + var security = GetBasicMutexSecurity(); + + using Mutex mutexNew = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); + using Mutex mutexExisting = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: false); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void Mutex_Create_BeyondMaxPathLength() + { + string name = new string('x', Interop.Kernel32.MAX_PATH + 100); + + if (PlatformDetection.IsFullFramework) + { + Assert.Throws(() => + { + using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); + }); + } + else + { + using Mutex mutex = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); + using Mutex openedByName = Mutex.OpenExisting(name); + Assert.NotNull(openedByName); + } + } + + [Theory] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true, MutexRights.FullControl, AccessControlType.Allow)] + [InlineData(true, MutexRights.FullControl, AccessControlType.Deny)] + [InlineData(true, MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, MutexRights.Modify, AccessControlType.Allow)] + [InlineData(true, MutexRights.Modify, AccessControlType.Deny)] + [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, MutexRights.FullControl, AccessControlType.Allow)] + [InlineData(false, MutexRights.FullControl, AccessControlType.Deny)] + [InlineData(false, MutexRights.Synchronize, AccessControlType.Allow)] + [InlineData(false, MutexRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, MutexRights.Modify, AccessControlType.Allow)] + [InlineData(false, MutexRights.Modify, AccessControlType.Deny)] + public void Mutex_Create_SpecificParameters(bool initiallyOwned, MutexRights rights, AccessControlType accessControl) + { + var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); + CreateAndVerifyMutex(initiallyOwned, GetRandomName(), security, expectedCreatedNew: true); + + } + + private MutexSecurity GetBasicMutexSecurity() + { + return GetMutexSecurity( + WellKnownSidType.BuiltinUsersSid, + MutexRights.FullControl, + AccessControlType.Allow); + } + + private MutexSecurity GetMutexSecurity(WellKnownSidType sid, MutexRights rights, AccessControlType accessControl) + { + var security = new MutexSecurity(); + SecurityIdentifier identity = new SecurityIdentifier(sid, null); + var accessRule = new MutexAccessRule(identity, rights, accessControl); + security.AddAccessRule(accessRule); + return security; + } + + private Mutex CreateAndVerifyMutex(bool initiallyOwned, string name, MutexSecurity expectedSecurity, bool expectedCreatedNew) + { + Mutex mutex = MutexAcl.Create(initiallyOwned, name, out bool createdNew, expectedSecurity); + Assert.NotNull(mutex); + Assert.Equal(createdNew, expectedCreatedNew); + + if (expectedSecurity != null) + { + MutexSecurity actualSecurity = mutex.GetAccessControl(); + VerifyMutexSecurity(expectedSecurity, actualSecurity); + } + + return mutex; + } + + private void VerifyMutexSecurity(MutexSecurity expectedSecurity, MutexSecurity actualSecurity) + { + Assert.Equal(typeof(MutexRights), expectedSecurity.AccessRightType); + Assert.Equal(typeof(MutexRights), actualSecurity.AccessRightType); + + List expectedAccessRules = expectedSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) + .Cast().ToList(); + + List actualAccessRules = actualSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) + .Cast().ToList(); + + Assert.Equal(expectedAccessRules.Count, actualAccessRules.Count); + if (expectedAccessRules.Count > 0) + { + Assert.All(expectedAccessRules, actualAccessRule => + { + int count = expectedAccessRules.Count(expectedAccessRule => AreAccessRulesEqual(expectedAccessRule, actualAccessRule)); + Assert.True(count > 0); + }); + } + } + + private bool AreAccessRulesEqual(MutexAccessRule expectedRule, MutexAccessRule actualRule) + { + return + expectedRule.AccessControlType == actualRule.AccessControlType && + expectedRule.MutexRights == actualRule.MutexRights && + expectedRule.InheritanceFlags == actualRule.InheritanceFlags && + expectedRule.PropagationFlags == actualRule.PropagationFlags; + } + } +} diff --git a/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj b/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj index 223103bf6c9e..8e31f48f3125 100644 --- a/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj +++ b/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj @@ -5,6 +5,8 @@ + + diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 5b341e5450c5..066b246fcd71 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -2,20 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; -using System.Linq; using System.Security.AccessControl; -using System.Security.Principal; using Xunit; namespace System.Threading.Tests { public static class ThreadingAclExtensionsTests { - #region Test methods - - #region Existence tests - [Fact] [PlatformSpecific(TestPlatforms.Windows)] // APIs not supported on Unix public static void ExistenceTest_Windows() @@ -52,192 +45,5 @@ public static void ExistenceTest_Unix() Assert.Throws(() => m.GetAccessControl()); Assert.Throws(() => m.SetAccessControl(new MutexSecurity())); } - - #endregion - - #region Mutex - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_NullSecurity() - { - using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), expectedSecurity: null, expectedCreatedNew: true); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_NullName() - { - using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, name: null, GetBasicMutexSecurity(), expectedCreatedNew: true); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_NullNameMultipleNew() - { - string name = string.Empty; - var security = GetBasicMutexSecurity(); - - using Mutex mutex1 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); - using Mutex mutex2 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_EmptyName() - { - using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, string.Empty, GetBasicMutexSecurity(), expectedCreatedNew: true); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_EmptyNameMultipleNew() - { - string name = string.Empty; - var security = GetBasicMutexSecurity(); - - using Mutex mutex1 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); - using Mutex mutex2 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_CreateNewExisting() - { - string name = GetRandomName(); - var security = GetBasicMutexSecurity(); - - using Mutex mutexNew = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); - using Mutex mutexExisting = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: false); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_BeyondMaxPathLength() - { - string name = new string('x', Interop.Kernel32.MAX_PATH + 100); - - if (PlatformDetection.IsFullFramework) - { - Assert.Throws(() => - { - using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); - }); - } - else - { - using Mutex mutex = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); - using Mutex openedByName = Mutex.OpenExisting(name); - Assert.NotNull(openedByName); - } - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void Mutex_Create_BasicSecurity() - { - var security = GetBasicMutexSecurity(); - using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), security, expectedCreatedNew: true); - } - - [Theory] - [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, MutexRights.FullControl, AccessControlType.Allow)] - [InlineData(true, MutexRights.FullControl, AccessControlType.Deny)] - [InlineData(true, MutexRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, MutexRights.Synchronize, AccessControlType.Deny)] - [InlineData(true, MutexRights.Modify, AccessControlType.Allow)] - [InlineData(true, MutexRights.Modify, AccessControlType.Deny)] - [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, MutexRights.Modify | MutexRights.Synchronize, AccessControlType.Deny)] - [InlineData(false, MutexRights.FullControl, AccessControlType.Allow)] - [InlineData(false, MutexRights.FullControl, AccessControlType.Deny)] - [InlineData(false, MutexRights.Synchronize, AccessControlType.Allow)] - [InlineData(false, MutexRights.Synchronize, AccessControlType.Deny)] - [InlineData(false, MutexRights.Modify, AccessControlType.Allow)] - [InlineData(false, MutexRights.Modify, AccessControlType.Deny)] - public static void Mutex_Create_SpecificParameters(bool initiallyOwned, MutexRights rights, AccessControlType accessControl) - { - var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); - CreateAndVerifyMutex(initiallyOwned, GetRandomName(), security, expectedCreatedNew: true); - - } - - #endregion - - #endregion - - #region Helper methods - - private static string GetRandomName() - { - return Guid.NewGuid().ToString("N"); - } - - private static MutexSecurity GetBasicMutexSecurity() - { - return GetMutexSecurity( - WellKnownSidType.BuiltinUsersSid, - MutexRights.FullControl, - AccessControlType.Allow); - } - - private static MutexSecurity GetMutexSecurity(WellKnownSidType sid, MutexRights rights, AccessControlType accessControl) - { - var security = new MutexSecurity(); - SecurityIdentifier identity = new SecurityIdentifier(sid, null); - var accessRule = new MutexAccessRule(identity, rights, accessControl); - security.AddAccessRule(accessRule); - return security; - } - - private static Mutex CreateAndVerifyMutex(bool initiallyOwned, string name, MutexSecurity expectedSecurity, bool expectedCreatedNew) - { - Mutex mutex = MutexAcl.Create(initiallyOwned, name, out bool createdNew, expectedSecurity); - Assert.NotNull(mutex); - Assert.Equal(createdNew, expectedCreatedNew); - - if (expectedSecurity != null) - { - MutexSecurity actualSecurity = mutex.GetAccessControl(); - VerifyMutexSecurity(expectedSecurity, actualSecurity); - } - - return mutex; - } - - private static void VerifyMutexSecurity(MutexSecurity expectedSecurity, MutexSecurity actualSecurity) - { - Assert.Equal(typeof(MutexRights), expectedSecurity.AccessRightType); - Assert.Equal(typeof(MutexRights), actualSecurity.AccessRightType); - - List expectedAccessRules = expectedSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) - .Cast().ToList(); - - List actualAccessRules = actualSecurity.GetAccessRules(includeExplicit: true, includeInherited: false, typeof(SecurityIdentifier)) - .Cast().ToList(); - - Assert.Equal(expectedAccessRules.Count, actualAccessRules.Count); - if (expectedAccessRules.Count > 0) - { - Assert.All(expectedAccessRules, actualAccessRule => - { - int count = expectedAccessRules.Count(expectedAccessRule => AreAccessRulesEqual(expectedAccessRule, actualAccessRule)); - Assert.True(count > 0); - }); - } - } - - private static bool AreAccessRulesEqual(MutexAccessRule expectedRule, MutexAccessRule actualRule) - { - return - expectedRule.AccessControlType == actualRule.AccessControlType && - expectedRule.MutexRights == actualRule.MutexRights && - expectedRule.InheritanceFlags == actualRule.InheritanceFlags && - expectedRule.PropagationFlags == actualRule.PropagationFlags; - } - - - #endregion } } From 039c30f9acbf70ec7f48837527ab51a3dad91651 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez Date: Tue, 5 Nov 2019 10:25:44 -0800 Subject: [PATCH 7/8] Dispose test mutexes, remove windows attribute, add license headers --- .../src/System/Threading/MutexAcl.cs | 2 +- .../tests/AclTests.cs | 6 +++++- .../tests/MutexAclTests.cs | 19 +++++++++---------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs index 597ba1761c7b..e1f2fbb191b4 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs @@ -16,7 +16,7 @@ public static class MutexAcl /// The optional name of the system mutex. If this argument is set to or , a local mutex is created. /// When this method returns, this argument is always set to if a local mutex is created; that is, when is or . If has a valid non-empty value, this argument is set to when the system mutex is created, or it is set to if an existing system mutex is found with that name. This parameter is passed uninitialized. /// The optional mutex access control security to apply. - /// An object that represents a system mutex. + /// An object that represents a system mutex, if named, or a local mutex, if nameless. /// .NET Framework only: The length of the name exceeds the maximum limit. /// A mutex handle with system-wide cannot be created. A mutex handle of a different type might have the same name. public static unsafe Mutex Create(bool initiallyOwned, string name, out bool createdNew, MutexSecurity mutexSecurity) diff --git a/src/System.Threading.AccessControl/tests/AclTests.cs b/src/System.Threading.AccessControl/tests/AclTests.cs index 40f9ae11aa22..496d8e76a746 100644 --- a/src/System.Threading.AccessControl/tests/AclTests.cs +++ b/src/System.Threading.AccessControl/tests/AclTests.cs @@ -1,4 +1,8 @@ -namespace System.Threading.Tests +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace System.Threading.Tests { public class AclTests { diff --git a/src/System.Threading.AccessControl/tests/MutexAclTests.cs b/src/System.Threading.AccessControl/tests/MutexAclTests.cs index 14a1185aaff2..f51f09e04e6a 100644 --- a/src/System.Threading.AccessControl/tests/MutexAclTests.cs +++ b/src/System.Threading.AccessControl/tests/MutexAclTests.cs @@ -1,4 +1,8 @@ -using System.Collections.Generic; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections.Generic; using System.Linq; using System.Security.AccessControl; using System.Security.Principal; @@ -9,16 +13,14 @@ namespace System.Threading.Tests public class MutexAclTests : AclTests { [Fact] - [PlatformSpecific(TestPlatforms.Windows)] public void Mutex_Create_NullSecurity() { - using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), expectedSecurity: null, expectedCreatedNew: true); + CreateAndVerifyMutex(initiallyOwned: true, GetRandomName(), expectedSecurity: null, expectedCreatedNew: true).Dispose(); } [Theory] [InlineData(null)] [InlineData("")] - [PlatformSpecific(TestPlatforms.Windows)] public void Mutex_Create_NameMultipleNew(string name) { var security = GetBasicMutexSecurity(); @@ -28,7 +30,6 @@ public void Mutex_Create_NameMultipleNew(string name) } [Fact] - [PlatformSpecific(TestPlatforms.Windows)] public void Mutex_Create_CreateNewExisting() { string name = GetRandomName(); @@ -39,7 +40,6 @@ public void Mutex_Create_CreateNewExisting() } [Fact] - [PlatformSpecific(TestPlatforms.Windows)] public void Mutex_Create_BeyondMaxPathLength() { string name = new string('x', Interop.Kernel32.MAX_PATH + 100); @@ -48,19 +48,18 @@ public void Mutex_Create_BeyondMaxPathLength() { Assert.Throws(() => { - using Mutex _ = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); + CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true).Dispose(); }); } else { - using Mutex mutex = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); + using Mutex created = CreateAndVerifyMutex(initiallyOwned: true, name, GetBasicMutexSecurity(), expectedCreatedNew: true); using Mutex openedByName = Mutex.OpenExisting(name); Assert.NotNull(openedByName); } } [Theory] - [PlatformSpecific(TestPlatforms.Windows)] [InlineData(true, MutexRights.FullControl, AccessControlType.Allow)] [InlineData(true, MutexRights.FullControl, AccessControlType.Deny)] [InlineData(true, MutexRights.Synchronize, AccessControlType.Allow)] @@ -78,7 +77,7 @@ public void Mutex_Create_BeyondMaxPathLength() public void Mutex_Create_SpecificParameters(bool initiallyOwned, MutexRights rights, AccessControlType accessControl) { var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); - CreateAndVerifyMutex(initiallyOwned, GetRandomName(), security, expectedCreatedNew: true); + CreateAndVerifyMutex(initiallyOwned, GetRandomName(), security, expectedCreatedNew: true).Dispose(); } From 219e3f97ce9fb0c0fef1006ca4379ec6b05c06b7 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 5 Nov 2019 12:07:54 -0800 Subject: [PATCH 8/8] suggestion to not use var --- .../src/System/Threading/MutexAcl.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs index e1f2fbb191b4..0d94b09b48de 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs @@ -45,8 +45,8 @@ public static unsafe Mutex Create(bool initiallyOwned, string name, out bool cre ValidateMutexHandle(handle, name, out createdNew); - var mutex = new Mutex(initiallyOwned); - var old = mutex.SafeWaitHandle; + Mutex mutex = new Mutex(initiallyOwned); + SafeWaitHandle old = mutex.SafeWaitHandle; mutex.SafeWaitHandle = handle; old.Dispose();