From 75737069417d172d65ee34ab3a8e2f03f49e7a81 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez Date: Tue, 29 Oct 2019 22:18:32 -0700 Subject: [PATCH 01/23] Add EventWaitHandle creation extension method that takes an ACL --- .../Windows/Kernel32/Interop.CreateEvent.cs | 19 +++ .../ref/System.Threading.AccessControl.cs | 4 + .../src/Resources/Strings.resx | 3 + .../System/Threading/EventWaitHandleAcl.cs | 110 ++++++++++++++++++ .../Threading/EventWaitHandleAcl.net46.cs | 21 ++++ 5 files changed, 157 insertions(+) create mode 100644 src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs create mode 100644 src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs create mode 100644 src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs diff --git a/src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs b/src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs new file mode 100644 index 000000000000..0a609ed44fa1 --- /dev/null +++ b/src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs @@ -0,0 +1,19 @@ +// 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.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +internal partial class Interop +{ + internal partial class Kernel32 + { + [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + internal static extern SafeWaitHandle CreateEvent( + ref SECURITY_ATTRIBUTES lpSecurityAttributes, + bool isManualReset, + bool initialState, + string name); + } +} diff --git a/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs b/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs index 0298eb449501..f9a0ecf2c7f0 100644 --- a/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs +++ b/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs @@ -151,6 +151,10 @@ public static class MutexAcl { public static System.Threading.Mutex Create(bool initiallyOwned, string name, out bool createdNew, System.Security.AccessControl.MutexSecurity mutexSecurity) { throw null; } } + public static class EventWaitHandleAcl + { + public static System.Threading.EventWaitHandle Create(bool initialState, System.Threading.EventResetMode mode, string name, out bool createdNew, System.Security.AccessControl.EventWaitHandleSecurity eventSecurity) { throw null; } + } public static class SemaphoreAcl { public static System.Threading.Semaphore Create(int initialCount, int maximumCount, string name, out bool createdNew, System.Security.AccessControl.SemaphoreSecurity semaphoreSecurity) { throw null; } diff --git a/src/System.Threading.AccessControl/src/Resources/Strings.resx b/src/System.Threading.AccessControl/src/Resources/Strings.resx index f676b7bfba52..e16169b4b8e9 100644 --- a/src/System.Threading.AccessControl/src/Resources/Strings.resx +++ b/src/System.Threading.AccessControl/src/Resources/Strings.resx @@ -129,6 +129,9 @@ Argument cannot be null or empty. + + Argument cannot be null or empty. + The length of the name exceeds the maximum limit. diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs new file mode 100644 index 000000000000..f02fc0212705 --- /dev/null +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -0,0 +1,110 @@ +// 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 EventWaitHandleAcl + { + /// Creates a new instance, ensuring it is created with the specified event security. + /// to set the initial state to signaled if the named event is created as a result of this call; to set it to nonsignaled. + /// One of the enum values that determines whether the event resets automatically or manually. + /// The name of a system-wide synchronization event. + /// When this method returns, it is set to if a local event was created (that is, if name is or an empty string) or if the specified named system event was created; if the specified named system event already existed. This parameter is passed uninitialized. + /// The Windows access control security to apply. + /// + /// The length of the name exceeds the maximum limit. + /// is . + /// The enum value was out of legal range. + /// An with system-wide name cannot be created. An of a different type might have the same name. + public static EventWaitHandle Create(bool initialState, EventResetMode mode, string name, out bool createdNew, EventWaitHandleSecurity eventSecurity) + { + if (name != null && name.Length > Interop.Kernel32.MAX_PATH) + { + throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, name)); + } + + if (eventSecurity == null) + { + throw new ArgumentNullException(nameof(eventSecurity)); + } + + bool isManualReset; + switch (mode) + { + case EventResetMode.ManualReset: + isManualReset = true; + break; + case EventResetMode.AutoReset: + isManualReset = false; + break; + default: + throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)); + }; + + SafeWaitHandle handle = CreateWaitHandle(initialState, isManualReset, name, out createdNew, eventSecurity); + + try + { + // This constructor is private + // return new EventWaitHandle(handle); + return null; + } + catch + { + handle.Dispose(); + throw; + } + } + + + private static unsafe SafeWaitHandle CreateWaitHandle(bool initialState, bool isManualReset, string name, out bool createdNew, EventWaitHandleSecurity security) + { + SafeWaitHandle handle; + + fixed (byte* pSecurityDescriptor = security.GetSecurityDescriptorBinaryForm()) + { + var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES + { + nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES), + lpSecurityDescriptor = (IntPtr)pSecurityDescriptor + }; + + handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); + ValidateHandle(handle, name, out createdNew); + } + + return handle; + } + + private static void ValidateHandle(SafeWaitHandle handle, string name, out bool createdNew) + { + createdNew = false; + + if (handle.IsInvalid) + { + // We call this in netfx. Is it still needed at this point? + handle.SetHandleAsInvalid(); + + int errorCode = Marshal.GetLastWin32Error(); + + if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE && string.IsNullOrEmpty(name)) + { + throw new WaitHandleCannotBeOpenedException( + SR.Format(SR.WaitHandleCannotBeOpenedException_InvalidHandle, name)); + } + else if (errorCode != Interop.Errors.ERROR_ALREADY_EXISTS) + { + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); + } + + createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; + } + } + } +} diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs new file mode 100644 index 000000000000..2b8e65d72448 --- /dev/null +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs @@ -0,0 +1,21 @@ +// 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 EventWaitHandleAcl + { + public static EventWaitHandle Create( + bool initialState, + EventResetMode mode, + string name, + out bool createdNew, + EventWaitHandleSecurity eventSecurity) + { + return new EventWaitHandle(initialState, mode, name, out createdNew, eventSecurity); + } + } +} From 9b0c0c08e36a0afab0fec55895501f2f6f456b46 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 30 Oct 2019 14:50:03 -0700 Subject: [PATCH 02/23] Call OpenExisting, add basic unit tests, move MAX_PATH in csproj for all Windows platforms, ensure same exceptions are thrown in both netcore and netfx. --- .../Kernel32/Interop.EventWaitHandle.cs | 12 ++- .../Windows/Kernel32/Interop.CreateEvent.cs | 19 ---- .../src/System.Threading.AccessControl.csproj | 1 + .../System/Threading/EventWaitHandleAcl.cs | 101 ++++++++++-------- .../Threading/EventWaitHandleAcl.net46.cs | 17 +++ .../tests/ThreadingAclExtensionsTests.cs | 60 +++++++++++ 6 files changed, 140 insertions(+), 70 deletions(-) delete mode 100644 src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs diff --git a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs index dabe8c2dc2bd..104eff339e8b 100644 --- a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs +++ b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.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; @@ -13,16 +14,19 @@ internal static partial class Kernel32 internal const uint CREATE_EVENT_INITIAL_SET = 0x2; internal const uint CREATE_EVENT_MANUAL_RESET = 0x1; - [DllImport(Interop.Libraries.Kernel32, SetLastError = true)] + [DllImport(Libraries.Kernel32, SetLastError = true)] internal static extern bool SetEvent(SafeWaitHandle handle); - [DllImport(Interop.Libraries.Kernel32, SetLastError = true)] + [DllImport(Libraries.Kernel32, SetLastError = true)] internal static extern bool ResetEvent(SafeWaitHandle handle); - [DllImport(Interop.Libraries.Kernel32, EntryPoint = "CreateEventExW", SetLastError = true, CharSet = CharSet.Unicode)] + [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventExW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle CreateEventEx(IntPtr lpSecurityAttributes, string? name, uint flags, uint desiredAccess); - [DllImport(Interop.Libraries.Kernel32, EntryPoint = "OpenEventW", SetLastError = true, CharSet = CharSet.Unicode)] + [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, bool isManualReset, bool initialState, string? name); + + [DllImport(Libraries.Kernel32, EntryPoint = "OpenEventW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle OpenEvent(uint desiredAccess, bool inheritHandle, string name); } } diff --git a/src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs b/src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs deleted file mode 100644 index 0a609ed44fa1..000000000000 --- a/src/Common/src/Interop/Windows/Kernel32/Interop.CreateEvent.cs +++ /dev/null @@ -1,19 +0,0 @@ -// 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.Runtime.InteropServices; -using Microsoft.Win32.SafeHandles; - -internal partial class Interop -{ - internal partial class Kernel32 - { - [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern SafeWaitHandle CreateEvent( - ref SECURITY_ATTRIBUTES lpSecurityAttributes, - bool isManualReset, - bool initialState, - string name); - } -} diff --git a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj index 23200dd3920c..789afc8018cc 100644 --- a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj +++ b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj @@ -13,6 +13,7 @@ + diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index f02fc0212705..08716182ae29 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -2,7 +2,9 @@ // 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.Diagnostics; using System.IO; +using System.Net.Mail; using System.Runtime.InteropServices; using System.Security.AccessControl; using Microsoft.Win32.SafeHandles; @@ -11,7 +13,7 @@ namespace System.Threading { public static class EventWaitHandleAcl { - /// Creates a new instance, ensuring it is created with the specified event security. + /// Creates a new named instance, ensuring it is created with the specified event security. /// to set the initial state to signaled if the named event is created as a result of this call; to set it to nonsignaled. /// One of the enum values that determines whether the event resets automatically or manually. /// The name of a system-wide synchronization event. @@ -21,51 +23,41 @@ public static class EventWaitHandleAcl /// The length of the name exceeds the maximum limit. /// is . /// The enum value was out of legal range. - /// An with system-wide name cannot be created. An of a different type might have the same name. + /// Could not find a part of the path specified in . + /// An event with the provided was not found. + /// -or- + /// An with system-wide name cannot be created. An of a different type might have the same name. public static EventWaitHandle Create(bool initialState, EventResetMode mode, string name, out bool createdNew, EventWaitHandleSecurity eventSecurity) { - if (name != null && name.Length > Interop.Kernel32.MAX_PATH) + if (string.IsNullOrEmpty(name)) { - throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, 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 (eventSecurity == null) { throw new ArgumentNullException(nameof(eventSecurity)); } - bool isManualReset; - switch (mode) + bool isManualReset = mode switch { - case EventResetMode.ManualReset: - isManualReset = true; - break; - case EventResetMode.AutoReset: - isManualReset = false; - break; - default: - throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)); + EventResetMode.ManualReset => true, + EventResetMode.AutoReset => false, + _ => throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)) }; - SafeWaitHandle handle = CreateWaitHandle(initialState, isManualReset, name, out createdNew, eventSecurity); - - try - { - // This constructor is private - // return new EventWaitHandle(handle); - return null; - } - catch - { - handle.Dispose(); - throw; - } + return CreateEventWaitHandle(initialState, isManualReset, name, out createdNew, eventSecurity); } - - private static unsafe SafeWaitHandle CreateWaitHandle(bool initialState, bool isManualReset, string name, out bool createdNew, EventWaitHandleSecurity security) + private static unsafe EventWaitHandle CreateEventWaitHandle(bool initialState, bool isManualReset, string name, out bool createdNew, EventWaitHandleSecurity security) { - SafeWaitHandle handle; + Debug.Assert(!string.IsNullOrEmpty(name)); + Debug.Assert(security != null); + + EventWaitHandle eventHandle = null; fixed (byte* pSecurityDescriptor = security.GetSecurityDescriptorBinaryForm()) { @@ -75,36 +67,51 @@ private static unsafe SafeWaitHandle CreateWaitHandle(bool initialState, bool is lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); + SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); ValidateHandle(handle, name, out createdNew); + + AggregateException aggEx = null; + + // Ensure we always close the handle regardless of the OpenExisting call result + try + { + eventHandle = EventWaitHandle.OpenExisting(name); + } + catch (Exception ex) + { + aggEx = new AggregateException(ex); + } + finally + { + handle.Dispose(); + } + + if (aggEx != null) + { + throw aggEx; + } } - return handle; + return eventHandle; } private static void ValidateHandle(SafeWaitHandle handle, string name, out bool createdNew) { - createdNew = false; + Debug.Assert(handle != null); + + int errorCode = Marshal.GetLastWin32Error(); if (handle.IsInvalid) { - // We call this in netfx. Is it still needed at this point? handle.SetHandleAsInvalid(); - int errorCode = Marshal.GetLastWin32Error(); + if (!string.IsNullOrEmpty(name) && errorCode == Interop.Errors.ERROR_INVALID_HANDLE) + throw new WaitHandleCannotBeOpenedException(SR.Format(SR.WaitHandleCannotBeOpenedException_InvalidHandle, name)); - if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE && string.IsNullOrEmpty(name)) - { - throw new WaitHandleCannotBeOpenedException( - SR.Format(SR.WaitHandleCannotBeOpenedException_InvalidHandle, name)); - } - else if (errorCode != Interop.Errors.ERROR_ALREADY_EXISTS) - { - throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); - } - - createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); } + + createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; } } } diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs index 2b8e65d72448..3779b8933359 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs @@ -15,6 +15,23 @@ public static EventWaitHandle Create( out bool createdNew, EventWaitHandleSecurity eventSecurity) { + 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 (eventSecurity == null) + { + throw new ArgumentNullException(nameof(eventSecurity)); + } + if (mode != EventResetMode.AutoReset && mode != EventResetMode.ManualReset) + { + throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)); + } + return new EventWaitHandle(initialState, mode, name, out createdNew, eventSecurity); } } diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 066b246fcd71..dfc9ddff1446 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -2,7 +2,9 @@ // 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.Diagnostics; using System.Security.AccessControl; +using System.Transactions; using Xunit; namespace System.Threading.Tests @@ -45,5 +47,63 @@ public static void ExistenceTest_Unix() Assert.Throws(() => m.GetAccessControl()); Assert.Throws(() => m.SetAccessControl(new MutexSecurity())); } + + #region EventWaitHandle + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_NullSecurity() + { + AssertExtensions.Throws("eventSecurity", () => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, "Test", out bool createdNew, eventSecurity: null); + }); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_InvalidName() + { + AssertExtensions.Throws("name", () => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew, new EventWaitHandleSecurity()); + }); + + AssertExtensions.Throws("name", () => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew, new EventWaitHandleSecurity()); + }); + + AssertExtensions.Throws("name", () => + { + string name = new string('X', Interop.Kernel32.MAX_PATH + 1); + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, new EventWaitHandleSecurity()); + }); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_InvalidMode() + { + AssertExtensions.Throws("mode", () => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, (EventResetMode)(-1), "name", out bool createdNew, new EventWaitHandleSecurity()); + }); + + AssertExtensions.Throws("mode", () => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, (EventResetMode)2, "name", out bool createdNew, new EventWaitHandleSecurity()); + }); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_DefaultSecurity() + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, "WhyIsThisUnauthorized", out bool createdNew, new EventWaitHandleSecurity()); + Assert.NotNull(eventHandle); + } + + #endregion } } From 2984404ad4a8ae6f0146f6bbd64c84b06fcba160 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 31 Oct 2019 10:55:01 -0700 Subject: [PATCH 03/23] Address suggestions: using for the created handle, simplify try finally, merge methods, remove an unnecessary Debug.Assert --- .../System/Threading/EventWaitHandleAcl.cs | 53 ++++++------------- .../tests/ThreadingAclExtensionsTests.cs | 2 +- 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 08716182ae29..5a2d0d0bac04 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -27,7 +27,7 @@ public static class EventWaitHandleAcl /// An event with the provided was not found. /// -or- /// An with system-wide name cannot be created. An of a different type might have the same name. - public static EventWaitHandle Create(bool initialState, EventResetMode mode, string name, out bool createdNew, EventWaitHandleSecurity eventSecurity) + public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mode, string name, out bool createdNew, EventWaitHandleSecurity eventSecurity) { if (string.IsNullOrEmpty(name)) { @@ -49,17 +49,7 @@ public static EventWaitHandle Create(bool initialState, EventResetMode mode, str _ => throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)) }; - return CreateEventWaitHandle(initialState, isManualReset, name, out createdNew, eventSecurity); - } - - private static unsafe EventWaitHandle CreateEventWaitHandle(bool initialState, bool isManualReset, string name, out bool createdNew, EventWaitHandleSecurity security) - { - Debug.Assert(!string.IsNullOrEmpty(name)); - Debug.Assert(security != null); - - EventWaitHandle eventHandle = null; - - fixed (byte* pSecurityDescriptor = security.GetSecurityDescriptorBinaryForm()) + fixed (byte* pSecurityDescriptor = eventSecurity.GetSecurityDescriptorBinaryForm()) { var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES { @@ -67,38 +57,25 @@ private static unsafe EventWaitHandle CreateEventWaitHandle(bool initialState, b lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); - ValidateHandle(handle, name, out createdNew); - - AggregateException aggEx = null; - - // Ensure we always close the handle regardless of the OpenExisting call result - try - { - eventHandle = EventWaitHandle.OpenExisting(name); - } - catch (Exception ex) - { - aggEx = new AggregateException(ex); - } - finally - { - handle.Dispose(); - } - - if (aggEx != null) + using (SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name)) { - throw aggEx; + ValidateHandle(handle, name, out createdNew); + + try + { + return EventWaitHandle.OpenExisting(name); + } + finally + { + // Close our handle as the EventWaitHandle will have it's own. + handle.Dispose(); + } } } - - return eventHandle; } private static void ValidateHandle(SafeWaitHandle handle, string name, out bool createdNew) { - Debug.Assert(handle != null); - int errorCode = Marshal.GetLastWin32Error(); if (handle.IsInvalid) @@ -111,7 +88,7 @@ private static void ValidateHandle(SafeWaitHandle handle, string name, out bool throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); } - createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; + createdNew = (errorCode != Interop.Errors.ERROR_ALREADY_EXISTS); } } } diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index dfc9ddff1446..51f85de283d8 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -100,7 +100,7 @@ public static void EventWaitHandle_Create_InvalidMode() [PlatformSpecific(TestPlatforms.Windows)] public static void EventWaitHandle_Create_DefaultSecurity() { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, "WhyIsThisUnauthorized", out bool createdNew, new EventWaitHandleSecurity()); + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, "MyHandle", out bool createdNew, new EventWaitHandleSecurity()); Assert.NotNull(eventHandle); } From ff6391efdc62ea0d3098fe9434ce3ef0f5fcd80d Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 31 Oct 2019 11:03:13 -0700 Subject: [PATCH 04/23] simplify using, reorganize PInvokes a bit, let VS format resx as it expects it, remove BOM from csproj --- .../System/Threading/EventWaitHandleAcl.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 5a2d0d0bac04..29f5a4501644 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -57,19 +57,18 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - using (SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name)) - { - ValidateHandle(handle, name, out createdNew); + using SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); + + ValidateHandle(handle, name, out createdNew); - try - { - return EventWaitHandle.OpenExisting(name); - } - finally - { - // Close our handle as the EventWaitHandle will have it's own. - handle.Dispose(); - } + try + { + return EventWaitHandle.OpenExisting(name); + } + finally + { + // Close our handle as the EventWaitHandle will have it's own. + handle.Dispose(); } } } From b518de9ccab171173ecd30e06cea6758ea302df7 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 31 Oct 2019 11:55:42 -0700 Subject: [PATCH 05/23] Add more unit tests --- .../Kernel32/Interop.EventWaitHandle.cs | 6 +- .../tests/ThreadingAclExtensionsTests.cs | 143 +++++++++++++++--- 2 files changed, 127 insertions(+), 22 deletions(-) diff --git a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs index 104eff339e8b..f78104033d2a 100644 --- a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs +++ b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs @@ -23,10 +23,10 @@ internal static partial class Kernel32 [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventExW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle CreateEventEx(IntPtr lpSecurityAttributes, string? name, uint flags, uint desiredAccess); - [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, bool isManualReset, bool initialState, string? name); - [DllImport(Libraries.Kernel32, EntryPoint = "OpenEventW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle OpenEvent(uint desiredAccess, bool inheritHandle, string name); + + [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, bool isManualReset, bool initialState, string? name); } } diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 51f85de283d8..804697f6bf93 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -2,8 +2,11 @@ // 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.Diagnostics; +using System.Linq; using System.Security.AccessControl; +using System.Security.Principal; using System.Transactions; using Xunit; @@ -11,6 +14,10 @@ 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() @@ -48,6 +55,8 @@ public static void ExistenceTest_Unix() Assert.Throws(() => m.SetAccessControl(new MutexSecurity())); } + #endregion + #region EventWaitHandle [Fact] @@ -60,48 +69,144 @@ public static void EventWaitHandle_Create_NullSecurity() }); } - [Fact] + [Theory] [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_InvalidName() + [InlineData(null)] + [InlineData("")] + public static void EventWaitHandle_Create_InvalidName(string name) { AssertExtensions.Throws("name", () => { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew, new EventWaitHandleSecurity()); - }); - - AssertExtensions.Throws("name", () => - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew, new EventWaitHandleSecurity()); + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); }); + } + [Fact] + public static void EventWaitHandle_Create_BeyondMaxLengthName() + { AssertExtensions.Throws("name", () => { string name = new string('X', Interop.Kernel32.MAX_PATH + 1); - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, new EventWaitHandleSecurity()); + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); }); } - [Fact] + [Theory] [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_InvalidMode() + [InlineData((EventResetMode)int.MinValue)] + [InlineData((EventResetMode)(-1))] + [InlineData((EventResetMode)2)] + [InlineData((EventResetMode)int.MaxValue)] + public static void EventWaitHandle_Create_InvalidMode(EventResetMode mode) { AssertExtensions.Throws("mode", () => { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, (EventResetMode)(-1), "name", out bool createdNew, new EventWaitHandleSecurity()); - }); - - AssertExtensions.Throws("mode", () => - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, (EventResetMode)2, "name", out bool createdNew, new EventWaitHandleSecurity()); + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, mode, "name", out bool createdNew, GetBasicEventWaitHandleSecurity()); }); } [Fact] [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_DefaultSecurity() + public static void EventWaitHandle_Create_BasicSecurity() + { + var security = GetBasicEventWaitHandleSecurity(); + VerifyEventWaitHandle(security); + } + + [Theory] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true, EventResetMode.AutoReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(false, EventResetMode.AutoReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(true, EventResetMode.ManualReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(false, EventResetMode.ManualReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(true, EventResetMode.AutoReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(false, EventResetMode.AutoReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(true, EventResetMode.ManualReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(false, EventResetMode.ManualReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, AccessControlType accessControl, int rights) { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, "MyHandle", out bool createdNew, new EventWaitHandleSecurity()); + var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, (EventWaitHandleRights)rights, accessControl); + if (accessControl == AccessControlType.Deny) + { + Assert.Throws(() => + { + VerifyEventWaitHandle(initialState, mode, "MyName", security); + }); + } + else + { + VerifyEventWaitHandle(initialState, mode, "MyName", security); + } + } + + #endregion + + #endregion + + #region Helper methods + + private static EventWaitHandleSecurity GetBasicEventWaitHandleSecurity() + { + return GetEventWaitHandleSecurity( + WellKnownSidType.BuiltinUsersSid, + EventWaitHandleRights.Synchronize | EventWaitHandleRights.Modify | EventWaitHandleRights.FullControl, + AccessControlType.Allow); + } + + private static EventWaitHandleSecurity GetEventWaitHandleSecurity(WellKnownSidType sid, EventWaitHandleRights rights, AccessControlType accessControl) + { + var security = new EventWaitHandleSecurity(); + SecurityIdentifier identity = new SecurityIdentifier(sid, null); + var accessRule = new EventWaitHandleAccessRule(identity, rights, accessControl); + security.AddAccessRule(accessRule); + return security; + } + + private static void VerifyEventWaitHandle(EventWaitHandleSecurity security) + { + VerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, "DefaultEventName", security); + } + + private static void VerifyEventWaitHandle(bool initialState, EventResetMode mode, string name, EventWaitHandleSecurity expectedSecurity) + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); + Assert.NotNull(eventHandle); + + EventWaitHandleSecurity actualSecurity = eventHandle.GetAccessControl(); + + VerifyEventWaitHandleSecurity(expectedSecurity, actualSecurity); + } + + private static void VerifyEventWaitHandleSecurity(EventWaitHandleSecurity expectedSecurity, EventWaitHandleSecurity actualSecurity) + { + Assert.Equal(typeof(EventWaitHandleRights), expectedSecurity.AccessRightType); + Assert.Equal(typeof(EventWaitHandleRights), 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(EventWaitHandleAccessRule expectedRule, EventWaitHandleAccessRule actualRule) + { + return + expectedRule.AccessControlType == actualRule.AccessControlType && + expectedRule.EventWaitHandleRights == actualRule.EventWaitHandleRights && + expectedRule.InheritanceFlags == actualRule.InheritanceFlags && + expectedRule.PropagationFlags == actualRule.PropagationFlags; } #endregion From 240bbc70eb28500e5aa50d03207a5a33129fecae Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 31 Oct 2019 15:53:42 -0700 Subject: [PATCH 06/23] Save basic access rights in a constant, generate name randomly --- .../tests/ThreadingAclExtensionsTests.cs | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 804697f6bf93..f87295e48be1 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -3,17 +3,20 @@ // See the LICENSE file in the project root for more information. using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Security.AccessControl; using System.Security.Principal; -using System.Transactions; using Xunit; namespace System.Threading.Tests { public static class ThreadingAclExtensionsTests { + // Dec: 34603010 + // Hex: 0x2100002 + // As predefined here: https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/EventWaitHandle.Windows.cs + private const int BasicAccessRights = Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE; + #region Test methods #region Existence tests @@ -110,33 +113,25 @@ public static void EventWaitHandle_Create_InvalidMode(EventResetMode mode) public static void EventWaitHandle_Create_BasicSecurity() { var security = GetBasicEventWaitHandleSecurity(); - VerifyEventWaitHandle(security); + CreateAndVerifyEventWaitHandle(security); } [Theory] [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, EventResetMode.AutoReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] - [InlineData(false, EventResetMode.AutoReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] - [InlineData(true, EventResetMode.ManualReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] - [InlineData(false, EventResetMode.ManualReset, AccessControlType.Allow, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] - [InlineData(true, EventResetMode.AutoReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] - [InlineData(false, EventResetMode.AutoReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] - [InlineData(true, EventResetMode.ManualReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] - [InlineData(false, EventResetMode.ManualReset, AccessControlType.Deny, Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE)] + [InlineData(true, EventResetMode.AutoReset, AccessControlType.Allow, BasicAccessRights)] + [InlineData(false, EventResetMode.AutoReset, AccessControlType.Allow, BasicAccessRights)] + [InlineData(true, EventResetMode.ManualReset, AccessControlType.Allow, BasicAccessRights)] + [InlineData(false, EventResetMode.ManualReset, AccessControlType.Allow, BasicAccessRights)] + [InlineData(true, EventResetMode.AutoReset, AccessControlType.Deny, BasicAccessRights)] + [InlineData(false, EventResetMode.AutoReset, AccessControlType.Deny, BasicAccessRights)] + [InlineData(true, EventResetMode.ManualReset, AccessControlType.Deny, BasicAccessRights)] + [InlineData(false, EventResetMode.ManualReset, AccessControlType.Deny, BasicAccessRights)] + public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, AccessControlType accessControl, int rights) { var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, (EventWaitHandleRights)rights, accessControl); - if (accessControl == AccessControlType.Deny) - { - Assert.Throws(() => - { - VerifyEventWaitHandle(initialState, mode, "MyName", security); - }); - } - else - { - VerifyEventWaitHandle(initialState, mode, "MyName", security); - } + CreateAndVerifyEventWaitHandle(initialState, mode, security); + } #endregion @@ -145,11 +140,16 @@ public static void EventWaitHandle_Create_SpecificParameters(bool initialState, #region Helper methods + private static string GetRandomNameMaxLength() + { + return Guid.NewGuid().ToString("N"); + } + private static EventWaitHandleSecurity GetBasicEventWaitHandleSecurity() { return GetEventWaitHandleSecurity( WellKnownSidType.BuiltinUsersSid, - EventWaitHandleRights.Synchronize | EventWaitHandleRights.Modify | EventWaitHandleRights.FullControl, + (EventWaitHandleRights)BasicAccessRights, AccessControlType.Allow); } @@ -162,16 +162,20 @@ private static EventWaitHandleSecurity GetEventWaitHandleSecurity(WellKnownSidTy return security; } - private static void VerifyEventWaitHandle(EventWaitHandleSecurity security) + private static void CreateAndVerifyEventWaitHandle(EventWaitHandleSecurity security) { - VerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, "DefaultEventName", security); + + CreateAndVerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, security); } - private static void VerifyEventWaitHandle(bool initialState, EventResetMode mode, string name, EventWaitHandleSecurity expectedSecurity) + private static void CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, EventWaitHandleSecurity expectedSecurity) { + string name = GetRandomNameMaxLength(); + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); Assert.NotNull(eventHandle); + Assert.True(createdNew); EventWaitHandleSecurity actualSecurity = eventHandle.GetAccessControl(); From ce079a58cc3d7904144abbb4bd1ced40b6c518cc Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 31 Oct 2019 18:05:49 -0700 Subject: [PATCH 07/23] Use FullControl (EVENT_ALL_ACCESS); update unit tests to use EventWaitHandleRights --- .../Kernel32/Interop.EventWaitHandle.cs | 4 +-- .../System/Threading/EventWaitHandleAcl.cs | 14 +++++------ .../tests/ThreadingAclExtensionsTests.cs | 25 ++++++------------- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs index f78104033d2a..2d6a875547e0 100644 --- a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs +++ b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs @@ -26,7 +26,7 @@ internal static partial class Kernel32 [DllImport(Libraries.Kernel32, EntryPoint = "OpenEventW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle OpenEvent(uint desiredAccess, bool inheritHandle, string name); - [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, bool isManualReset, bool initialState, string? name); + [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventExW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, string? name, uint flags, uint desiredAccess); } } diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 29f5a4501644..3feff06be695 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -19,7 +19,7 @@ public static class EventWaitHandleAcl /// The name of a system-wide synchronization event. /// When this method returns, it is set to if a local event was created (that is, if name is or an empty string) or if the specified named system event was created; if the specified named system event already existed. This parameter is passed uninitialized. /// The Windows access control security to apply. - /// + /// An object that represents the named event wait handle. /// The length of the name exceeds the maximum limit. /// is . /// The enum value was out of legal range. @@ -42,12 +42,12 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo throw new ArgumentNullException(nameof(eventSecurity)); } - bool isManualReset = mode switch + uint eventFlags = initialState ? Interop.Kernel32.CREATE_EVENT_INITIAL_SET : 0; + + if (mode == EventResetMode.ManualReset) { - EventResetMode.ManualReset => true, - EventResetMode.AutoReset => false, - _ => throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)) - }; + eventFlags |= (uint)Interop.Kernel32.CREATE_EVENT_MANUAL_RESET; + } fixed (byte* pSecurityDescriptor = eventSecurity.GetSecurityDescriptorBinaryForm()) { @@ -57,7 +57,7 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - using SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); + using SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, name, eventFlags, (uint)EventWaitHandleRights.FullControl); ValidateHandle(handle, name, out createdNew); diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index f87295e48be1..fa390097ba17 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -12,11 +12,6 @@ namespace System.Threading.Tests { public static class ThreadingAclExtensionsTests { - // Dec: 34603010 - // Hex: 0x2100002 - // As predefined here: https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/EventWaitHandle.Windows.cs - private const int BasicAccessRights = Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.EVENT_MODIFY_STATE; - #region Test methods #region Existence tests @@ -118,18 +113,14 @@ public static void EventWaitHandle_Create_BasicSecurity() [Theory] [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, EventResetMode.AutoReset, AccessControlType.Allow, BasicAccessRights)] - [InlineData(false, EventResetMode.AutoReset, AccessControlType.Allow, BasicAccessRights)] - [InlineData(true, EventResetMode.ManualReset, AccessControlType.Allow, BasicAccessRights)] - [InlineData(false, EventResetMode.ManualReset, AccessControlType.Allow, BasicAccessRights)] - [InlineData(true, EventResetMode.AutoReset, AccessControlType.Deny, BasicAccessRights)] - [InlineData(false, EventResetMode.AutoReset, AccessControlType.Deny, BasicAccessRights)] - [InlineData(true, EventResetMode.ManualReset, AccessControlType.Deny, BasicAccessRights)] - [InlineData(false, EventResetMode.ManualReset, AccessControlType.Deny, BasicAccessRights)] - - public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, AccessControlType accessControl, int rights) + [InlineData(true, EventResetMode.AutoReset, AccessControlType.Allow, EventWaitHandleRights.FullControl)] + [InlineData(true, EventResetMode.ManualReset, AccessControlType.Allow, EventWaitHandleRights.FullControl)] + [InlineData(true, EventResetMode.AutoReset, AccessControlType.Deny, EventWaitHandleRights.FullControl)] + [InlineData(true, EventResetMode.ManualReset, AccessControlType.Deny, EventWaitHandleRights.FullControl)] + + public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, AccessControlType accessControl, EventWaitHandleRights rights) { - var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, (EventWaitHandleRights)rights, accessControl); + var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); CreateAndVerifyEventWaitHandle(initialState, mode, security); } @@ -149,7 +140,7 @@ private static EventWaitHandleSecurity GetBasicEventWaitHandleSecurity() { return GetEventWaitHandleSecurity( WellKnownSidType.BuiltinUsersSid, - (EventWaitHandleRights)BasicAccessRights, + EventWaitHandleRights.FullControl, AccessControlType.Allow); } From e0cf7ccff52e10bac93dc3b6d7ed3b7d997c29b9 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 1 Nov 2019 11:36:28 -0700 Subject: [PATCH 08/23] Apply suggestion of creating an EWH, then replacing its SWH. --- .../Kernel32/Interop.EventWaitHandle.cs | 4 +- .../System/Threading/EventWaitHandleAcl.cs | 39 ++++++++----------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs index 2d6a875547e0..f78104033d2a 100644 --- a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs +++ b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs @@ -26,7 +26,7 @@ internal static partial class Kernel32 [DllImport(Libraries.Kernel32, EntryPoint = "OpenEventW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle OpenEvent(uint desiredAccess, bool inheritHandle, string name); - [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventExW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, string? name, uint flags, uint desiredAccess); + [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, bool isManualReset, bool initialState, string? name); } } diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 3feff06be695..011ec8e264c1 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -13,10 +13,10 @@ namespace System.Threading { public static class EventWaitHandleAcl { - /// Creates a new named instance, ensuring it is created with the specified event security. - /// to set the initial state to signaled if the named event is created as a result of this call; to set it to nonsignaled. + /// Creates a new instance, ensuring it is created with the specified event security. + /// to set the initial state to signaled if the named event is created as a result of this call; to set it to non-signaled. /// One of the enum values that determines whether the event resets automatically or manually. - /// The name of a system-wide synchronization event. + /// The name, if the event is a system-wide synchronization event; otherwise, or an empty string. /// When this method returns, it is set to if a local event was created (that is, if name is or an empty string) or if the specified named system event was created; if the specified named system event already existed. This parameter is passed uninitialized. /// The Windows access control security to apply. /// An object that represents the named event wait handle. @@ -24,15 +24,11 @@ public static class EventWaitHandleAcl /// is . /// The enum value was out of legal range. /// Could not find a part of the path specified in . - /// An event with the provided was not found. + /// A system-wide synchronization event with the provided was not found. /// -or- /// An with system-wide name cannot be created. An of a different type might have the same name. public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mode, string name, out bool createdNew, EventWaitHandleSecurity eventSecurity) { - 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)); @@ -42,12 +38,12 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo throw new ArgumentNullException(nameof(eventSecurity)); } - uint eventFlags = initialState ? Interop.Kernel32.CREATE_EVENT_INITIAL_SET : 0; - - if (mode == EventResetMode.ManualReset) + bool isManualReset = mode switch { - eventFlags |= (uint)Interop.Kernel32.CREATE_EVENT_MANUAL_RESET; - } + EventResetMode.ManualReset => true, + EventResetMode.AutoReset => false, + _ => throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)) + }; fixed (byte* pSecurityDescriptor = eventSecurity.GetSecurityDescriptorBinaryForm()) { @@ -57,19 +53,16 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - using SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, name, eventFlags, (uint)EventWaitHandleRights.FullControl); + using SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); ValidateHandle(handle, name, out createdNew); - try - { - return EventWaitHandle.OpenExisting(name); - } - finally - { - // Close our handle as the EventWaitHandle will have it's own. - handle.Dispose(); - } + var ewh = new EventWaitHandle(initialState, mode); + var old = ewh.SafeWaitHandle; + ewh.SafeWaitHandle = handle; + old.Dispose(); + + return ewh; } } From 09f5a7b1c9eea9aeef150f3652adcaa51648c595 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 1 Nov 2019 11:54:38 -0700 Subject: [PATCH 09/23] Remove using causing chaos, add more unit tests, remove null string unit test. --- .../System/Threading/EventWaitHandleAcl.cs | 2 +- .../tests/ThreadingAclExtensionsTests.cs | 36 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 011ec8e264c1..1b1c1838b337 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -53,7 +53,7 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - using SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); + SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); ValidateHandle(handle, name, out createdNew); diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index fa390097ba17..96ca67fde878 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -67,18 +67,6 @@ public static void EventWaitHandle_Create_NullSecurity() }); } - [Theory] - [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(null)] - [InlineData("")] - public static void EventWaitHandle_Create_InvalidName(string name) - { - AssertExtensions.Throws("name", () => - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); - }); - } - [Fact] public static void EventWaitHandle_Create_BeyondMaxLengthName() { @@ -113,12 +101,24 @@ public static void EventWaitHandle_Create_BasicSecurity() [Theory] [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, EventResetMode.AutoReset, AccessControlType.Allow, EventWaitHandleRights.FullControl)] - [InlineData(true, EventResetMode.ManualReset, AccessControlType.Allow, EventWaitHandleRights.FullControl)] - [InlineData(true, EventResetMode.AutoReset, AccessControlType.Deny, EventWaitHandleRights.FullControl)] - [InlineData(true, EventResetMode.ManualReset, AccessControlType.Deny, EventWaitHandleRights.FullControl)] - - public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, AccessControlType accessControl, EventWaitHandleRights rights) + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + + public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, EventWaitHandleRights rights, AccessControlType accessControl) { var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); CreateAndVerifyEventWaitHandle(initialState, mode, security); From 7fb95e3fe827f98fbd78e389efd4acbc8692536f Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 1 Nov 2019 11:55:59 -0700 Subject: [PATCH 10/23] spacing --- .../tests/ThreadingAclExtensionsTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 96ca67fde878..96c43a07c723 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -117,7 +117,6 @@ public static void EventWaitHandle_Create_BasicSecurity() [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, EventWaitHandleRights rights, AccessControlType accessControl) { var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); From 66b1d7aa7b2c791d40d3d4194fa5273ef40c4716 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 1 Nov 2019 15:24:20 -0700 Subject: [PATCH 11/23] Address comments --- .../Kernel32/Interop.EventWaitHandle.cs | 3 - .../System/Threading/EventWaitHandleAcl.cs | 26 ++-- .../Threading/EventWaitHandleAcl.net46.cs | 9 +- .../tests/ThreadingAclExtensionsTests.cs | 121 +++++++++++++++--- 4 files changed, 121 insertions(+), 38 deletions(-) diff --git a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs index f78104033d2a..c3adfadb53d5 100644 --- a/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs +++ b/src/Common/src/CoreLib/Interop/Windows/Kernel32/Interop.EventWaitHandle.cs @@ -25,8 +25,5 @@ internal static partial class Kernel32 [DllImport(Libraries.Kernel32, EntryPoint = "OpenEventW", SetLastError = true, CharSet = CharSet.Unicode)] internal static extern SafeWaitHandle OpenEvent(uint desiredAccess, bool inheritHandle, string name); - - [DllImport(Libraries.Kernel32, EntryPoint = "CreateEventW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern SafeWaitHandle CreateEvent(ref SECURITY_ATTRIBUTES lpSecurityAttributes, bool isManualReset, bool initialState, string? name); } } diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 1b1c1838b337..508f7485c0c9 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -20,8 +20,10 @@ public static class EventWaitHandleAcl /// When this method returns, it is set to if a local event was created (that is, if name is or an empty string) or if the specified named system event was created; if the specified named system event already existed. This parameter is passed uninitialized. /// The Windows access control security to apply. /// An object that represents the named event wait handle. - /// The length of the name exceeds the maximum limit. - /// is . + /// is . + /// -or- + /// .NET Framework only: The length is beyond MAX_PATH (260 characters). + /// /// The enum value was out of legal range. /// Could not find a part of the path specified in . /// A system-wide synchronization event with the provided was not found. @@ -29,21 +31,21 @@ public static class EventWaitHandleAcl /// An with system-wide name cannot be created. An of a different type might have the same name. public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mode, string name, out bool createdNew, EventWaitHandleSecurity eventSecurity) { - if (name.Length > Interop.Kernel32.MAX_PATH) - { - throw new ArgumentException(SR.Format(SR.Argument_WaitHandleNameTooLong, name), nameof(name)); - } if (eventSecurity == null) { throw new ArgumentNullException(nameof(eventSecurity)); } - bool isManualReset = mode switch + if (mode != EventResetMode.AutoReset && mode != EventResetMode.ManualReset) { - EventResetMode.ManualReset => true, - EventResetMode.AutoReset => false, - _ => throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)) - }; + throw new ArgumentOutOfRangeException(nameof(mode)); + } + + uint eventFlags = initialState ? Interop.Kernel32.CREATE_EVENT_INITIAL_SET : 0; + if (mode == EventResetMode.ManualReset) + { + eventFlags |= (uint)Interop.Kernel32.CREATE_EVENT_MANUAL_RESET; + } fixed (byte* pSecurityDescriptor = eventSecurity.GetSecurityDescriptorBinaryForm()) { @@ -53,7 +55,7 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - SafeWaitHandle handle = Interop.Kernel32.CreateEvent(ref secAttrs, isManualReset, initialState, name); + SafeWaitHandle handle = Interop.Kernel32.CreateEventEx((IntPtr)(&secAttrs), name, eventFlags, (uint)EventWaitHandleRights.FullControl); ValidateHandle(handle, name, out createdNew); diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs index 3779b8933359..827e2ae72a58 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs @@ -15,18 +15,11 @@ public static EventWaitHandle Create( out bool createdNew, EventWaitHandleSecurity eventSecurity) { - 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 (eventSecurity == null) { throw new ArgumentNullException(nameof(eventSecurity)); } + if (mode != EventResetMode.AutoReset && mode != EventResetMode.ManualReset) { throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)); diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 96c43a07c723..4863063210b1 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Generic; +using System.IO; using System.Linq; using System.Security.AccessControl; using System.Security.Principal; @@ -68,15 +69,107 @@ public static void EventWaitHandle_Create_NullSecurity() } [Fact] - public static void EventWaitHandle_Create_BeyondMaxLengthName() + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_NullName() + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle); + Assert.True(createdNew); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_NullNameMultipleNew() + { + using EventWaitHandle eventHandle1 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew1, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle1); + Assert.True(createdNew1); + + using EventWaitHandle eventHandle2 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew2, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle2); + Assert.True(createdNew2); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_EmptyName() + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle); + Assert.True(createdNew); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_EmptyNameMultipleNew() + { + using EventWaitHandle eventHandle1 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew1, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle1); + Assert.True(createdNew1); + + using EventWaitHandle eventHandle2 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew2, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle2); + Assert.True(createdNew2); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_CreateNewExisting() { - AssertExtensions.Throws("name", () => + string name = GetRandomName(); + + using EventWaitHandle eventHandleNew = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew1, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandleNew); + Assert.True(createdNew1); + + using EventWaitHandle eventHandleExisting = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew2, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandleExisting); + Assert.False(createdNew2); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_GlobalPrefixNameNotFound() + { + Assert.Throws(() => { - string name = new string('X', Interop.Kernel32.MAX_PATH + 1); - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); + string prefixedName = @"GLOBAL\" + GetRandomName(); + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, prefixedName, out bool createdNew, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle); + Assert.True(createdNew); }); } + // The documentation says MAX_PATH is the length limit for name, but it won't throw any errors: + // https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createeventexw + // The .NET Core constructors for EventWaitHandle do not throw on name longer than MAX_LENGTH, so the extension method should match the behavior: + // https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/EventWaitHandle.Windows.cs,20 + // The .NET Framework constructor throws: + // https://referencesource.microsoft.com/#mscorlib/system/threading/eventwaithandle.cs,59 + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public static void EventWaitHandle_Create_BeyondMaxPathLength() + { + string name = new string('x', Interop.Kernel32.MAX_PATH + 100); + + if (PlatformDetection.IsFullFramework) + { + Assert.Throws(() => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); + }); + } + else + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); + Assert.NotNull(eventHandle); + Assert.True(createdNew); + + using EventWaitHandle openedByName = EventWaitHandle.OpenExisting(name); + Assert.NotNull(openedByName); + } + } + [Theory] [PlatformSpecific(TestPlatforms.Windows)] [InlineData((EventResetMode)int.MinValue)] @@ -107,16 +200,16 @@ public static void EventWaitHandle_Create_BasicSecurity() [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, EventWaitHandleRights rights, AccessControlType accessControl) { var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); @@ -130,7 +223,7 @@ public static void EventWaitHandle_Create_SpecificParameters(bool initialState, #region Helper methods - private static string GetRandomNameMaxLength() + private static string GetRandomName() { return Guid.NewGuid().ToString("N"); } @@ -154,16 +247,14 @@ private static EventWaitHandleSecurity GetEventWaitHandleSecurity(WellKnownSidTy private static void CreateAndVerifyEventWaitHandle(EventWaitHandleSecurity security) { - CreateAndVerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, security); } private static void CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, EventWaitHandleSecurity expectedSecurity) { - string name = GetRandomNameMaxLength(); + string name = GetRandomName(); using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); - Assert.NotNull(eventHandle); Assert.True(createdNew); @@ -197,10 +288,10 @@ private static void VerifyEventWaitHandleSecurity(EventWaitHandleSecurity expect private static bool AreAccessRulesEqual(EventWaitHandleAccessRule expectedRule, EventWaitHandleAccessRule actualRule) { return - expectedRule.AccessControlType == actualRule.AccessControlType && + expectedRule.AccessControlType == actualRule.AccessControlType && expectedRule.EventWaitHandleRights == actualRule.EventWaitHandleRights && - expectedRule.InheritanceFlags == actualRule.InheritanceFlags && - expectedRule.PropagationFlags == actualRule.PropagationFlags; + expectedRule.InheritanceFlags == actualRule.InheritanceFlags && + expectedRule.PropagationFlags == actualRule.PropagationFlags; } #endregion From 42adc3017d6cb60485f89bcaed1ccdf6ebab374c Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 1 Nov 2019 15:46:08 -0700 Subject: [PATCH 12/23] Remove unnecessary checks/exceptions, update unit tests --- .../System/Threading/EventWaitHandleAcl.cs | 4 +-- .../Threading/EventWaitHandleAcl.net46.cs | 10 ------ .../tests/ThreadingAclExtensionsTests.cs | 34 ++++++++----------- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 508f7485c0c9..953283fde276 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -33,7 +33,7 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo { if (eventSecurity == null) { - throw new ArgumentNullException(nameof(eventSecurity)); + return new EventWaitHandle(initialState, mode, name, out createdNew); } if (mode != EventResetMode.AutoReset && mode != EventResetMode.ManualReset) @@ -44,7 +44,7 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo uint eventFlags = initialState ? Interop.Kernel32.CREATE_EVENT_INITIAL_SET : 0; if (mode == EventResetMode.ManualReset) { - eventFlags |= (uint)Interop.Kernel32.CREATE_EVENT_MANUAL_RESET; + eventFlags |= Interop.Kernel32.CREATE_EVENT_MANUAL_RESET; } fixed (byte* pSecurityDescriptor = eventSecurity.GetSecurityDescriptorBinaryForm()) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs index 827e2ae72a58..2b8e65d72448 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.net46.cs @@ -15,16 +15,6 @@ public static EventWaitHandle Create( out bool createdNew, EventWaitHandleSecurity eventSecurity) { - if (eventSecurity == null) - { - throw new ArgumentNullException(nameof(eventSecurity)); - } - - if (mode != EventResetMode.AutoReset && mode != EventResetMode.ManualReset) - { - throw new ArgumentOutOfRangeException(nameof(mode), SR.Format(SR.ArgumentOutOfRange_Enum)); - } - return new EventWaitHandle(initialState, mode, name, out createdNew, eventSecurity); } } diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 4863063210b1..0b43bc556348 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -62,19 +62,14 @@ public static void ExistenceTest_Unix() [PlatformSpecific(TestPlatforms.Windows)] public static void EventWaitHandle_Create_NullSecurity() { - AssertExtensions.Throws("eventSecurity", () => - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, "Test", out bool createdNew, eventSecurity: null); - }); + CreateAndVerifyEventWaitHandle(null); } [Fact] [PlatformSpecific(TestPlatforms.Windows)] public static void EventWaitHandle_Create_NullName() { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle); - Assert.True(createdNew); + CreateAndVerifyEventWaitHandle(true, EventResetMode.AutoReset, null, GetBasicEventWaitHandleSecurity()); } [Fact] @@ -94,9 +89,7 @@ public static void EventWaitHandle_Create_NullNameMultipleNew() [PlatformSpecific(TestPlatforms.Windows)] public static void EventWaitHandle_Create_EmptyName() { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle); - Assert.True(createdNew); + CreateAndVerifyEventWaitHandle(true, EventResetMode.AutoReset, string.Empty, GetBasicEventWaitHandleSecurity()); } [Fact] @@ -134,9 +127,7 @@ public static void EventWaitHandle_Create_GlobalPrefixNameNotFound() Assert.Throws(() => { string prefixedName = @"GLOBAL\" + GetRandomName(); - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, prefixedName, out bool createdNew, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle); - Assert.True(createdNew); + CreateAndVerifyEventWaitHandle(true, EventResetMode.AutoReset, prefixedName, GetBasicEventWaitHandleSecurity()); }); } @@ -247,20 +238,25 @@ private static EventWaitHandleSecurity GetEventWaitHandleSecurity(WellKnownSidTy private static void CreateAndVerifyEventWaitHandle(EventWaitHandleSecurity security) { - CreateAndVerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, security); + CreateAndVerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, GetRandomName(), security); } - private static void CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, EventWaitHandleSecurity expectedSecurity) { - string name = GetRandomName(); + CreateAndVerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, GetRandomName(), expectedSecurity); + } + + private static void CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, string name, EventWaitHandleSecurity expectedSecurity) + { using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); Assert.NotNull(eventHandle); Assert.True(createdNew); - EventWaitHandleSecurity actualSecurity = eventHandle.GetAccessControl(); - - VerifyEventWaitHandleSecurity(expectedSecurity, actualSecurity); + if (expectedSecurity != null) + { + EventWaitHandleSecurity actualSecurity = eventHandle.GetAccessControl(); + VerifyEventWaitHandleSecurity(expectedSecurity, actualSecurity); + } } private static void VerifyEventWaitHandleSecurity(EventWaitHandleSecurity expectedSecurity, EventWaitHandleSecurity actualSecurity) From 6b6daa209bc138c512bb4d2d2ea9b30b4cb73316 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 4 Nov 2019 11:13:42 -0800 Subject: [PATCH 13/23] Fix netfx x86 ut failure - different exception for mode validation. --- .../src/System/Threading/EventWaitHandleAcl.cs | 8 +++++--- .../tests/ThreadingAclExtensionsTests.cs | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 953283fde276..f2b422c219be 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -2,9 +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. -using System.Diagnostics; using System.IO; -using System.Net.Mail; using System.Runtime.InteropServices; using System.Security.AccessControl; using Microsoft.Win32.SafeHandles; @@ -55,7 +53,11 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo lpSecurityDescriptor = (IntPtr)pSecurityDescriptor }; - SafeWaitHandle handle = Interop.Kernel32.CreateEventEx((IntPtr)(&secAttrs), name, eventFlags, (uint)EventWaitHandleRights.FullControl); + SafeWaitHandle handle = Interop.Kernel32.CreateEventEx( + (IntPtr)(&secAttrs), + name, + eventFlags, + (uint)EventWaitHandleRights.FullControl); ValidateHandle(handle, name, out createdNew); diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index 0b43bc556348..c1f35b2629cc 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -169,10 +169,20 @@ public static void EventWaitHandle_Create_BeyondMaxPathLength() [InlineData((EventResetMode)int.MaxValue)] public static void EventWaitHandle_Create_InvalidMode(EventResetMode mode) { - AssertExtensions.Throws("mode", () => + if (PlatformDetection.IsFullFramework) { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, mode, "name", out bool createdNew, GetBasicEventWaitHandleSecurity()); - }); + Assert.Throws(() => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, mode, "name", out bool createdNew, GetBasicEventWaitHandleSecurity()); + }); + } + else + { + Assert.Throws("mode", () => + { + using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, mode, "name", out bool createdNew, GetBasicEventWaitHandleSecurity()); + }); + } } [Fact] From 166da5cf5645cab0782adb8b2eff774a19a13e11 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 4 Nov 2019 12:57:01 -0800 Subject: [PATCH 14/23] Small documentation fix --- .../src/System/Threading/EventWaitHandleAcl.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index f2b422c219be..663e9033f7e5 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -11,7 +11,7 @@ namespace System.Threading { public static class EventWaitHandleAcl { - /// Creates a new instance, ensuring it is created with the specified event security. + /// Gets or creates an instance, allowing a instance to be optionally specified to set it during the event creation. /// to set the initial state to signaled if the named event is created as a result of this call; to set it to non-signaled. /// One of the enum values that determines whether the event resets automatically or manually. /// The name, if the event is a system-wide synchronization event; otherwise, or an empty string. @@ -27,6 +27,7 @@ public static class EventWaitHandleAcl /// A system-wide synchronization event with the provided was not found. /// -or- /// An with system-wide name cannot be created. An of a different type might have the same name. + /// If a `name` is passed and the system event already exists, the existing event is returned. If `name` is `null` or , a new local event is always created. public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mode, string name, out bool createdNew, EventWaitHandleSecurity eventSecurity) { if (eventSecurity == null) From f49035e7cd4224f0262d07cb78bb1d6230536ac4 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 4 Nov 2019 12:59:17 -0800 Subject: [PATCH 15/23] Remove documentation for exception thrown with null security --- .../src/System/Threading/EventWaitHandleAcl.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 663e9033f7e5..6b8a07ed5cda 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -18,10 +18,7 @@ public static class EventWaitHandleAcl /// When this method returns, it is set to if a local event was created (that is, if name is or an empty string) or if the specified named system event was created; if the specified named system event already existed. This parameter is passed uninitialized. /// The Windows access control security to apply. /// An object that represents the named event wait handle. - /// is . - /// -or- - /// .NET Framework only: The length is beyond MAX_PATH (260 characters). - /// + /// .NET Framework only: The length is beyond MAX_PATH (260 characters). /// The enum value was out of legal range. /// Could not find a part of the path specified in . /// A system-wide synchronization event with the provided was not found. From 0b0a5b6ec5c91ba3a31d3535dbfa084742944a55 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 4 Nov 2019 13:06:29 -0800 Subject: [PATCH 16/23] more documentation fixes --- .../src/System/Threading/EventWaitHandleAcl.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 6b8a07ed5cda..056a0df67182 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -15,8 +15,8 @@ public static class EventWaitHandleAcl /// to set the initial state to signaled if the named event is created as a result of this call; to set it to non-signaled. /// One of the enum values that determines whether the event resets automatically or manually. /// The name, if the event is a system-wide synchronization event; otherwise, or an empty string. - /// When this method returns, it is set to if a local event was created (that is, if name is or an empty string) or if the specified named system event was created; if the specified named system event already existed. This parameter is passed uninitialized. - /// The Windows access control security to apply. + /// When this method returns, this argument is always set to if a local event is created; that is, when is or . If has a valid, non-empty value, this argument is set to when the system event is created, or it is set to if an existing system event is found with that name. This parameter is passed uninitialized. + /// The optional Windows access control security to apply. /// An object that represents the named event wait handle. /// .NET Framework only: The length is beyond MAX_PATH (260 characters). /// The enum value was out of legal range. From aba83204b5fd621788a00ea405e4f5dee92c5263 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez Date: Mon, 4 Nov 2019 22:59:17 -0800 Subject: [PATCH 17/23] New file for unit tests --- .../tests/EventWaitHandleAclTests.cs | 272 ++++++++++++++++++ ...ystem.Threading.AccessControl.Tests.csproj | 1 + .../tests/ThreadingAclExtensionsTests.cs | 256 ----------------- 3 files changed, 273 insertions(+), 256 deletions(-) create mode 100644 src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs diff --git a/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs b/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs new file mode 100644 index 000000000000..8fe5b08ccf59 --- /dev/null +++ b/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs @@ -0,0 +1,272 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace System.Threading.Tests +{ + public class EventWaitHandleAclTests : AclTests + { + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void EventWaitHandle_Create_NullSecurity() + { + using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + initialState: true, + mode: EventResetMode.AutoReset, + name: GetRandomName(), + expectedSecurity: null, + expectedCreatedNew: true); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [PlatformSpecific(TestPlatforms.Windows)] + public void EventWaitHandle_Create_NameMultipleNew(string name) + { + var security = GetBasicEventWaitHandleSecurity(); + + using EventWaitHandle handle1 = CreateAndVerifyEventWaitHandle( + initialState: true, + mode: EventResetMode.AutoReset, + name, + security, + expectedCreatedNew: true); + + using EventWaitHandle handle2 = CreateAndVerifyEventWaitHandle( + initialState: true, + mode: EventResetMode.AutoReset, + name, + security, + expectedCreatedNew: true); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void EventWaitHandle_Create_CreateNewExisting() + { + string name = GetRandomName(); + var security = GetBasicEventWaitHandleSecurity(); + + using EventWaitHandle handle1 = CreateAndVerifyEventWaitHandle( + initialState: true, + mode: EventResetMode.AutoReset, + name, + security, + expectedCreatedNew: true); + + using EventWaitHandle handle2 = CreateAndVerifyEventWaitHandle( + initialState: true, + mode: EventResetMode.AutoReset, + name, + security, + expectedCreatedNew: false); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void EventWaitHandle_Create_GlobalPrefixNameNotFound() + { + Assert.Throws(() => + { + string prefixedName = @"GLOBAL\" + GetRandomName(); + + using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + initialState: true, + mode: EventResetMode.AutoReset, + prefixedName, + expectedSecurity: GetBasicEventWaitHandleSecurity(), + expectedCreatedNew: true); + }); + } + + // The documentation says MAX_PATH is the length limit for name, but it won't throw any errors: + // https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createeventexw + // The .NET Core constructors for EventWaitHandle do not throw on name longer than MAX_LENGTH, so the extension method should match the behavior: + // https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/EventWaitHandle.Windows.cs,20 + // The .NET Framework constructor throws: + // https://referencesource.microsoft.com/#mscorlib/system/threading/eventwaithandle.cs,59 + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void EventWaitHandle_Create_BeyondMaxPathLength() + { + string name = new string('x', Interop.Kernel32.MAX_PATH + 100); + var security = GetBasicEventWaitHandleSecurity(); + var mode = EventResetMode.AutoReset; + + if (PlatformDetection.IsFullFramework) + { + Assert.Throws(() => + { + using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + initialState: true, + mode, + name, + security, + expectedCreatedNew: true); + }); + } + else + { + using EventWaitHandle handle = CreateAndVerifyEventWaitHandle( + initialState: true, + mode, + name, + security, + expectedCreatedNew: true); + + using EventWaitHandle openedByName = EventWaitHandle.OpenExisting(name); + Assert.NotNull(openedByName); + } + } + + [Theory] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData((EventResetMode)int.MinValue)] + [InlineData((EventResetMode)(-1))] + [InlineData((EventResetMode)2)] + [InlineData((EventResetMode)int.MaxValue)] + public void EventWaitHandle_Create_InvalidMode(EventResetMode mode) + { + if (PlatformDetection.IsFullFramework) + { + Assert.Throws(() => + { + using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + initialState: true, + mode, + GetRandomName(), + GetBasicEventWaitHandleSecurity(), + expectedCreatedNew: true); + }); + } + else + { + Assert.Throws("mode", () => + { + using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + initialState: true, + mode, + GetRandomName(), + GetBasicEventWaitHandleSecurity(), + expectedCreatedNew: true); + }); + } + } + + [Theory] + [PlatformSpecific(TestPlatforms.Windows)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] + [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + public void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, EventWaitHandleRights rights, AccessControlType accessControl) + { + var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); + CreateAndVerifyEventWaitHandle( + initialState, + mode, + GetRandomName(), + security, + expectedCreatedNew: true); + + } + + private EventWaitHandleSecurity GetBasicEventWaitHandleSecurity() + { + return GetEventWaitHandleSecurity( + WellKnownSidType.BuiltinUsersSid, + EventWaitHandleRights.FullControl, + AccessControlType.Allow); + } + + private EventWaitHandleSecurity GetEventWaitHandleSecurity(WellKnownSidType sid, EventWaitHandleRights rights, AccessControlType accessControl) + { + var security = new EventWaitHandleSecurity(); + SecurityIdentifier identity = new SecurityIdentifier(sid, null); + var accessRule = new EventWaitHandleAccessRule(identity, rights, accessControl); + security.AddAccessRule(accessRule); + return security; + } + + private EventWaitHandle CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, string name, EventWaitHandleSecurity expectedSecurity, bool expectedCreatedNew) + { + EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); + Assert.NotNull(eventHandle); + Assert.Equal(expectedCreatedNew, createdNew); + + if (expectedSecurity != null) + { + EventWaitHandleSecurity actualSecurity = eventHandle.GetAccessControl(); + VerifyEventWaitHandleSecurity(expectedSecurity, actualSecurity); + } + + return eventHandle; + } + + private void VerifyEventWaitHandleSecurity(EventWaitHandleSecurity expectedSecurity, EventWaitHandleSecurity actualSecurity) + { + Assert.Equal(typeof(EventWaitHandleRights), expectedSecurity.AccessRightType); + Assert.Equal(typeof(EventWaitHandleRights), 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(EventWaitHandleAccessRule expectedRule, EventWaitHandleAccessRule actualRule) + { + return + expectedRule.AccessControlType == actualRule.AccessControlType && + expectedRule.EventWaitHandleRights == actualRule.EventWaitHandleRights && + 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 478d1d8db82e..3974171d2b43 100644 --- a/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj +++ b/src/System.Threading.AccessControl/tests/System.Threading.AccessControl.Tests.csproj @@ -6,6 +6,7 @@ + diff --git a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs index c1f35b2629cc..066b246fcd71 100644 --- a/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs +++ b/src/System.Threading.AccessControl/tests/ThreadingAclExtensionsTests.cs @@ -2,21 +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.IO; -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() @@ -53,253 +45,5 @@ public static void ExistenceTest_Unix() Assert.Throws(() => m.GetAccessControl()); Assert.Throws(() => m.SetAccessControl(new MutexSecurity())); } - - #endregion - - #region EventWaitHandle - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_NullSecurity() - { - CreateAndVerifyEventWaitHandle(null); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_NullName() - { - CreateAndVerifyEventWaitHandle(true, EventResetMode.AutoReset, null, GetBasicEventWaitHandleSecurity()); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_NullNameMultipleNew() - { - using EventWaitHandle eventHandle1 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew1, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle1); - Assert.True(createdNew1); - - using EventWaitHandle eventHandle2 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, null, out bool createdNew2, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle2); - Assert.True(createdNew2); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_EmptyName() - { - CreateAndVerifyEventWaitHandle(true, EventResetMode.AutoReset, string.Empty, GetBasicEventWaitHandleSecurity()); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_EmptyNameMultipleNew() - { - using EventWaitHandle eventHandle1 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew1, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle1); - Assert.True(createdNew1); - - using EventWaitHandle eventHandle2 = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, string.Empty, out bool createdNew2, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle2); - Assert.True(createdNew2); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_CreateNewExisting() - { - string name = GetRandomName(); - - using EventWaitHandle eventHandleNew = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew1, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandleNew); - Assert.True(createdNew1); - - using EventWaitHandle eventHandleExisting = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew2, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandleExisting); - Assert.False(createdNew2); - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_GlobalPrefixNameNotFound() - { - Assert.Throws(() => - { - string prefixedName = @"GLOBAL\" + GetRandomName(); - CreateAndVerifyEventWaitHandle(true, EventResetMode.AutoReset, prefixedName, GetBasicEventWaitHandleSecurity()); - }); - } - - // The documentation says MAX_PATH is the length limit for name, but it won't throw any errors: - // https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createeventexw - // The .NET Core constructors for EventWaitHandle do not throw on name longer than MAX_LENGTH, so the extension method should match the behavior: - // https://source.dot.net/#System.Private.CoreLib/shared/System/Threading/EventWaitHandle.Windows.cs,20 - // The .NET Framework constructor throws: - // https://referencesource.microsoft.com/#mscorlib/system/threading/eventwaithandle.cs,59 - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_BeyondMaxPathLength() - { - string name = new string('x', Interop.Kernel32.MAX_PATH + 100); - - if (PlatformDetection.IsFullFramework) - { - Assert.Throws(() => - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); - }); - } - else - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, EventResetMode.AutoReset, name, out bool createdNew, GetBasicEventWaitHandleSecurity()); - Assert.NotNull(eventHandle); - Assert.True(createdNew); - - using EventWaitHandle openedByName = EventWaitHandle.OpenExisting(name); - Assert.NotNull(openedByName); - } - } - - [Theory] - [PlatformSpecific(TestPlatforms.Windows)] - [InlineData((EventResetMode)int.MinValue)] - [InlineData((EventResetMode)(-1))] - [InlineData((EventResetMode)2)] - [InlineData((EventResetMode)int.MaxValue)] - public static void EventWaitHandle_Create_InvalidMode(EventResetMode mode) - { - if (PlatformDetection.IsFullFramework) - { - Assert.Throws(() => - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, mode, "name", out bool createdNew, GetBasicEventWaitHandleSecurity()); - }); - } - else - { - Assert.Throws("mode", () => - { - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState: true, mode, "name", out bool createdNew, GetBasicEventWaitHandleSecurity()); - }); - } - } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public static void EventWaitHandle_Create_BasicSecurity() - { - var security = GetBasicEventWaitHandleSecurity(); - CreateAndVerifyEventWaitHandle(security); - } - - [Theory] - [PlatformSpecific(TestPlatforms.Windows)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - public static void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, EventWaitHandleRights rights, AccessControlType accessControl) - { - var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); - CreateAndVerifyEventWaitHandle(initialState, mode, security); - - } - - #endregion - - #endregion - - #region Helper methods - - private static string GetRandomName() - { - return Guid.NewGuid().ToString("N"); - } - - private static EventWaitHandleSecurity GetBasicEventWaitHandleSecurity() - { - return GetEventWaitHandleSecurity( - WellKnownSidType.BuiltinUsersSid, - EventWaitHandleRights.FullControl, - AccessControlType.Allow); - } - - private static EventWaitHandleSecurity GetEventWaitHandleSecurity(WellKnownSidType sid, EventWaitHandleRights rights, AccessControlType accessControl) - { - var security = new EventWaitHandleSecurity(); - SecurityIdentifier identity = new SecurityIdentifier(sid, null); - var accessRule = new EventWaitHandleAccessRule(identity, rights, accessControl); - security.AddAccessRule(accessRule); - return security; - } - - private static void CreateAndVerifyEventWaitHandle(EventWaitHandleSecurity security) - { - CreateAndVerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, GetRandomName(), security); - } - private static void CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, EventWaitHandleSecurity expectedSecurity) - { - CreateAndVerifyEventWaitHandle(initialState: true, EventResetMode.AutoReset, GetRandomName(), expectedSecurity); - } - - private static void CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, string name, EventWaitHandleSecurity expectedSecurity) - { - - using EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); - Assert.NotNull(eventHandle); - Assert.True(createdNew); - - if (expectedSecurity != null) - { - EventWaitHandleSecurity actualSecurity = eventHandle.GetAccessControl(); - VerifyEventWaitHandleSecurity(expectedSecurity, actualSecurity); - } - } - - private static void VerifyEventWaitHandleSecurity(EventWaitHandleSecurity expectedSecurity, EventWaitHandleSecurity actualSecurity) - { - Assert.Equal(typeof(EventWaitHandleRights), expectedSecurity.AccessRightType); - Assert.Equal(typeof(EventWaitHandleRights), 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(EventWaitHandleAccessRule expectedRule, EventWaitHandleAccessRule actualRule) - { - return - expectedRule.AccessControlType == actualRule.AccessControlType && - expectedRule.EventWaitHandleRights == actualRule.EventWaitHandleRights && - expectedRule.InheritanceFlags == actualRule.InheritanceFlags && - expectedRule.PropagationFlags == actualRule.PropagationFlags; - } - - #endregion } } From 95af0e13d2a05785fb2d07aa835b056dbebb8091 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez Date: Tue, 5 Nov 2019 10:28:53 -0800 Subject: [PATCH 18/23] Dispose and remove windows only attributes in tests --- .../System/Threading/EventWaitHandleAcl.cs | 2 +- .../tests/EventWaitHandleAclTests.cs | 37 +++++++++---------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index 056a0df67182..ebad5f59db6f 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -17,7 +17,7 @@ public static class EventWaitHandleAcl /// The name, if the event is a system-wide synchronization event; otherwise, or an empty string. /// When this method returns, this argument is always set to if a local event is created; that is, when is or . If has a valid, non-empty value, this argument is set to when the system event is created, or it is set to if an existing system event is found with that name. This parameter is passed uninitialized. /// The optional Windows access control security to apply. - /// An object that represents the named event wait handle. + /// An object that represents a system event wait handle, if named, or a local event wait handle, if nameless. /// .NET Framework only: The length is beyond MAX_PATH (260 characters). /// The enum value was out of legal range. /// Could not find a part of the path specified in . diff --git a/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs b/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs index 8fe5b08ccf59..24e1cc77c3a6 100644 --- a/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs +++ b/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs @@ -1,4 +1,8 @@ -using System; +// 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; using System.Collections.Generic; using System.IO; using System.Linq; @@ -13,21 +17,19 @@ namespace System.Threading.Tests public class EventWaitHandleAclTests : AclTests { [Fact] - [PlatformSpecific(TestPlatforms.Windows)] public void EventWaitHandle_Create_NullSecurity() { - using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + CreateAndVerifyEventWaitHandle( initialState: true, mode: EventResetMode.AutoReset, name: GetRandomName(), expectedSecurity: null, - expectedCreatedNew: true); + expectedCreatedNew: true).Dispose(); } [Theory] [InlineData(null)] [InlineData("")] - [PlatformSpecific(TestPlatforms.Windows)] public void EventWaitHandle_Create_NameMultipleNew(string name) { var security = GetBasicEventWaitHandleSecurity(); @@ -48,7 +50,6 @@ public void EventWaitHandle_Create_NameMultipleNew(string name) } [Fact] - [PlatformSpecific(TestPlatforms.Windows)] public void EventWaitHandle_Create_CreateNewExisting() { string name = GetRandomName(); @@ -70,19 +71,18 @@ public void EventWaitHandle_Create_CreateNewExisting() } [Fact] - [PlatformSpecific(TestPlatforms.Windows)] public void EventWaitHandle_Create_GlobalPrefixNameNotFound() { Assert.Throws(() => { string prefixedName = @"GLOBAL\" + GetRandomName(); - using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + CreateAndVerifyEventWaitHandle( initialState: true, mode: EventResetMode.AutoReset, prefixedName, expectedSecurity: GetBasicEventWaitHandleSecurity(), - expectedCreatedNew: true); + expectedCreatedNew: true).Dispose(); }); } @@ -93,7 +93,6 @@ public void EventWaitHandle_Create_GlobalPrefixNameNotFound() // The .NET Framework constructor throws: // https://referencesource.microsoft.com/#mscorlib/system/threading/eventwaithandle.cs,59 [Fact] - [PlatformSpecific(TestPlatforms.Windows)] public void EventWaitHandle_Create_BeyondMaxPathLength() { string name = new string('x', Interop.Kernel32.MAX_PATH + 100); @@ -104,17 +103,17 @@ public void EventWaitHandle_Create_BeyondMaxPathLength() { Assert.Throws(() => { - using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + CreateAndVerifyEventWaitHandle( initialState: true, mode, name, security, - expectedCreatedNew: true); + expectedCreatedNew: true).Dispose(); }); } else { - using EventWaitHandle handle = CreateAndVerifyEventWaitHandle( + using EventWaitHandle created = CreateAndVerifyEventWaitHandle( initialState: true, mode, name, @@ -127,7 +126,6 @@ public void EventWaitHandle_Create_BeyondMaxPathLength() } [Theory] - [PlatformSpecific(TestPlatforms.Windows)] [InlineData((EventResetMode)int.MinValue)] [InlineData((EventResetMode)(-1))] [InlineData((EventResetMode)2)] @@ -138,30 +136,29 @@ public void EventWaitHandle_Create_InvalidMode(EventResetMode mode) { Assert.Throws(() => { - using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + CreateAndVerifyEventWaitHandle( initialState: true, mode, GetRandomName(), GetBasicEventWaitHandleSecurity(), - expectedCreatedNew: true); + expectedCreatedNew: true).Dispose(); }); } else { Assert.Throws("mode", () => { - using EventWaitHandle _ = CreateAndVerifyEventWaitHandle( + CreateAndVerifyEventWaitHandle( initialState: true, mode, GetRandomName(), GetBasicEventWaitHandleSecurity(), - expectedCreatedNew: true); + expectedCreatedNew: true).Dispose(); }); } } [Theory] - [PlatformSpecific(TestPlatforms.Windows)] [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] @@ -202,7 +199,7 @@ public void EventWaitHandle_Create_SpecificParameters(bool initialState, EventRe mode, GetRandomName(), security, - expectedCreatedNew: true); + expectedCreatedNew: true).Dispose(); } From be24175fe39fe23bfe7c06be9edb7c44114ee6ac Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 5 Nov 2019 12:09:13 -0800 Subject: [PATCH 19/23] suggestion to not use var --- .../src/System/Threading/EventWaitHandleAcl.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index ebad5f59db6f..f6a15ff7576d 100644 --- a/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -59,8 +59,8 @@ public static unsafe EventWaitHandle Create(bool initialState, EventResetMode mo ValidateHandle(handle, name, out createdNew); - var ewh = new EventWaitHandle(initialState, mode); - var old = ewh.SafeWaitHandle; + EventWaitHandle ewh = new EventWaitHandle(initialState, mode); + SafeWaitHandle old = ewh.SafeWaitHandle; ewh.SafeWaitHandle = handle; old.Dispose(); From 0e4eb4cd885536d80c032b3e2335ad6005d54b2c Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 7 Nov 2019 16:37:00 -0800 Subject: [PATCH 20/23] Remove duplicate resx after merge --- .../src/Resources/Strings.resx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/System.Threading.AccessControl/src/Resources/Strings.resx b/src/System.Threading.AccessControl/src/Resources/Strings.resx index e16169b4b8e9..5646718e9ba7 100644 --- a/src/System.Threading.AccessControl/src/Resources/Strings.resx +++ b/src/System.Threading.AccessControl/src/Resources/Strings.resx @@ -129,9 +129,6 @@ Argument cannot be null or empty. - - Argument cannot be null or empty. - The length of the name exceeds the maximum limit. @@ -183,4 +180,4 @@ 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 + From 3d62b9a4f424c09b0aac3dc19396074ae6c9a108 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 7 Nov 2019 16:53:45 -0800 Subject: [PATCH 21/23] Readd cs files lost during merge --- .../ref/System.Threading.AccessControl.cs | 8 ++++---- .../src/System.Threading.AccessControl.csproj | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs b/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs index f9a0ecf2c7f0..a4b5d31fb177 100644 --- a/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs +++ b/src/System.Threading.AccessControl/ref/System.Threading.AccessControl.cs @@ -147,14 +147,14 @@ 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; } - } public static class EventWaitHandleAcl { public static System.Threading.EventWaitHandle Create(bool initialState, System.Threading.EventResetMode mode, string name, out bool createdNew, System.Security.AccessControl.EventWaitHandleSecurity eventSecurity) { throw null; } } + public static class MutexAcl + { + public static System.Threading.Mutex Create(bool initiallyOwned, string name, out bool createdNew, System.Security.AccessControl.MutexSecurity mutexSecurity) { throw null; } + } public static class SemaphoreAcl { public static System.Threading.Semaphore Create(int initialCount, int maximumCount, string name, out bool createdNew, System.Security.AccessControl.SemaphoreSecurity semaphoreSecurity) { throw null; } diff --git a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj index 789afc8018cc..44aa39c63825 100644 --- a/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj +++ b/src/System.Threading.AccessControl/src/System.Threading.AccessControl.csproj @@ -23,6 +23,7 @@ + @@ -30,6 +31,7 @@ + From 0cf8663f7b0a498a8d5e7139d667f0a96fc445fe Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 7 Nov 2019 16:58:12 -0800 Subject: [PATCH 22/23] Remove resx modification since it was untouched for this PR --- src/System.Threading.AccessControl/src/Resources/Strings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Threading.AccessControl/src/Resources/Strings.resx b/src/System.Threading.AccessControl/src/Resources/Strings.resx index 5646718e9ba7..f676b7bfba52 100644 --- a/src/System.Threading.AccessControl/src/Resources/Strings.resx +++ b/src/System.Threading.AccessControl/src/Resources/Strings.resx @@ -180,4 +180,4 @@ 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 From bd2680f9e265e03eef9c7c3c3b72665219378123 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 7 Nov 2019 17:07:31 -0800 Subject: [PATCH 23/23] Address test comments --- .../tests/EventWaitHandleAclTests.cs | 80 +++++++------------ .../tests/MutexAclTests.cs | 10 +-- .../tests/SemaphoreAclTests.cs | 10 +-- 3 files changed, 39 insertions(+), 61 deletions(-) diff --git a/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs b/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs index 24e1cc77c3a6..ca02c306cff2 100644 --- a/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs +++ b/src/System.Threading.AccessControl/tests/EventWaitHandleAclTests.cs @@ -2,14 +2,11 @@ // 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; using System.Collections.Generic; using System.IO; using System.Linq; using System.Security.AccessControl; using System.Security.Principal; -using System.Text; -using System.Threading.Tasks; using Xunit; namespace System.Threading.Tests @@ -32,7 +29,7 @@ public void EventWaitHandle_Create_NullSecurity() [InlineData("")] public void EventWaitHandle_Create_NameMultipleNew(string name) { - var security = GetBasicEventWaitHandleSecurity(); + EventWaitHandleSecurity security = GetBasicEventWaitHandleSecurity(); using EventWaitHandle handle1 = CreateAndVerifyEventWaitHandle( initialState: true, @@ -53,7 +50,7 @@ public void EventWaitHandle_Create_NameMultipleNew(string name) public void EventWaitHandle_Create_CreateNewExisting() { string name = GetRandomName(); - var security = GetBasicEventWaitHandleSecurity(); + EventWaitHandleSecurity security = GetBasicEventWaitHandleSecurity(); using EventWaitHandle handle1 = CreateAndVerifyEventWaitHandle( initialState: true, @@ -73,11 +70,11 @@ public void EventWaitHandle_Create_CreateNewExisting() [Fact] public void EventWaitHandle_Create_GlobalPrefixNameNotFound() { + string prefixedName = @"GLOBAL\" + GetRandomName(); + Assert.Throws(() => { - string prefixedName = @"GLOBAL\" + GetRandomName(); - - CreateAndVerifyEventWaitHandle( + CreateEventWaitHandle( initialState: true, mode: EventResetMode.AutoReset, prefixedName, @@ -96,14 +93,14 @@ public void EventWaitHandle_Create_GlobalPrefixNameNotFound() public void EventWaitHandle_Create_BeyondMaxPathLength() { string name = new string('x', Interop.Kernel32.MAX_PATH + 100); - var security = GetBasicEventWaitHandleSecurity(); - var mode = EventResetMode.AutoReset; + EventWaitHandleSecurity security = GetBasicEventWaitHandleSecurity(); + EventResetMode mode = EventResetMode.AutoReset; if (PlatformDetection.IsFullFramework) { Assert.Throws(() => { - CreateAndVerifyEventWaitHandle( + CreateEventWaitHandle( initialState: true, mode, name, @@ -136,7 +133,7 @@ public void EventWaitHandle_Create_InvalidMode(EventResetMode mode) { Assert.Throws(() => { - CreateAndVerifyEventWaitHandle( + CreateEventWaitHandle( initialState: true, mode, GetRandomName(), @@ -148,7 +145,7 @@ public void EventWaitHandle_Create_InvalidMode(EventResetMode mode) { Assert.Throws("mode", () => { - CreateAndVerifyEventWaitHandle( + CreateEventWaitHandle( initialState: true, mode, GetRandomName(), @@ -158,42 +155,18 @@ public void EventWaitHandle_Create_InvalidMode(EventResetMode mode) } } + public static IEnumerable EventWaitHandle_Create_SpecificParameters_MemberData() => + from initialState in new[] { true, false } + from mode in new[] { EventResetMode.AutoReset, EventResetMode.ManualReset } + from rights in new[] { EventWaitHandleRights.FullControl, EventWaitHandleRights.Synchronize, EventWaitHandleRights.Modify, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize } + from accessControl in new[] { AccessControlType.Allow, AccessControlType.Deny } + select new object[] { initialState, mode, rights, accessControl }; + [Theory] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(true, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(false, EventResetMode.AutoReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Allow)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.FullControl, AccessControlType.Deny)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Synchronize, AccessControlType.Deny)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Allow)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify, AccessControlType.Deny)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Allow)] - [InlineData(false, EventResetMode.ManualReset, EventWaitHandleRights.Modify | EventWaitHandleRights.Synchronize, AccessControlType.Deny)] + [MemberData(nameof(EventWaitHandle_Create_SpecificParameters_MemberData))] public void EventWaitHandle_Create_SpecificParameters(bool initialState, EventResetMode mode, EventWaitHandleRights rights, AccessControlType accessControl) { - var security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); + EventWaitHandleSecurity security = GetEventWaitHandleSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); CreateAndVerifyEventWaitHandle( initialState, mode, @@ -213,18 +186,23 @@ private EventWaitHandleSecurity GetBasicEventWaitHandleSecurity() private EventWaitHandleSecurity GetEventWaitHandleSecurity(WellKnownSidType sid, EventWaitHandleRights rights, AccessControlType accessControl) { - var security = new EventWaitHandleSecurity(); + EventWaitHandleSecurity security = new EventWaitHandleSecurity(); SecurityIdentifier identity = new SecurityIdentifier(sid, null); - var accessRule = new EventWaitHandleAccessRule(identity, rights, accessControl); + EventWaitHandleAccessRule accessRule = new EventWaitHandleAccessRule(identity, rights, accessControl); security.AddAccessRule(accessRule); return security; } + private EventWaitHandle CreateEventWaitHandle(bool initialState, EventResetMode mode, string name, EventWaitHandleSecurity expectedSecurity, bool expectedCreatedNew) + { + EventWaitHandle handle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); + Assert.NotNull(handle); + Assert.Equal(expectedCreatedNew, createdNew); + return handle; + } private EventWaitHandle CreateAndVerifyEventWaitHandle(bool initialState, EventResetMode mode, string name, EventWaitHandleSecurity expectedSecurity, bool expectedCreatedNew) { - EventWaitHandle eventHandle = EventWaitHandleAcl.Create(initialState, mode, name, out bool createdNew, expectedSecurity); - Assert.NotNull(eventHandle); - Assert.Equal(expectedCreatedNew, createdNew); + EventWaitHandle eventHandle = CreateEventWaitHandle(initialState, mode, name, expectedSecurity, expectedCreatedNew); if (expectedSecurity != null) { diff --git a/src/System.Threading.AccessControl/tests/MutexAclTests.cs b/src/System.Threading.AccessControl/tests/MutexAclTests.cs index f51f09e04e6a..2b5bf0047442 100644 --- a/src/System.Threading.AccessControl/tests/MutexAclTests.cs +++ b/src/System.Threading.AccessControl/tests/MutexAclTests.cs @@ -23,7 +23,7 @@ public void Mutex_Create_NullSecurity() [InlineData("")] public void Mutex_Create_NameMultipleNew(string name) { - var security = GetBasicMutexSecurity(); + MutexSecurity security = GetBasicMutexSecurity(); using Mutex mutex1 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); using Mutex mutex2 = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); @@ -33,7 +33,7 @@ public void Mutex_Create_NameMultipleNew(string name) public void Mutex_Create_CreateNewExisting() { string name = GetRandomName(); - var security = GetBasicMutexSecurity(); + MutexSecurity security = GetBasicMutexSecurity(); using Mutex mutexNew = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: true); using Mutex mutexExisting = CreateAndVerifyMutex(initiallyOwned: true, name, security, expectedCreatedNew: false); @@ -76,7 +76,7 @@ public void Mutex_Create_BeyondMaxPathLength() [InlineData(false, MutexRights.Modify, AccessControlType.Deny)] public void Mutex_Create_SpecificParameters(bool initiallyOwned, MutexRights rights, AccessControlType accessControl) { - var security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); + MutexSecurity security = GetMutexSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); CreateAndVerifyMutex(initiallyOwned, GetRandomName(), security, expectedCreatedNew: true).Dispose(); } @@ -91,9 +91,9 @@ private MutexSecurity GetBasicMutexSecurity() private MutexSecurity GetMutexSecurity(WellKnownSidType sid, MutexRights rights, AccessControlType accessControl) { - var security = new MutexSecurity(); + MutexSecurity security = new MutexSecurity(); SecurityIdentifier identity = new SecurityIdentifier(sid, null); - var accessRule = new MutexAccessRule(identity, rights, accessControl); + MutexAccessRule accessRule = new MutexAccessRule(identity, rights, accessControl); security.AddAccessRule(accessRule); return security; } diff --git a/src/System.Threading.AccessControl/tests/SemaphoreAclTests.cs b/src/System.Threading.AccessControl/tests/SemaphoreAclTests.cs index 028cc316077e..970217fb8d34 100644 --- a/src/System.Threading.AccessControl/tests/SemaphoreAclTests.cs +++ b/src/System.Threading.AccessControl/tests/SemaphoreAclTests.cs @@ -48,7 +48,7 @@ public void Semaphore_Create_InvalidCounts(int initialCount, int maximumCount) [InlineData("")] public void Semaphore_Create_NameMultipleNew(string name) { - var security = GetBasicSemaphoreSecurity(); + SemaphoreSecurity security = GetBasicSemaphoreSecurity(); bool expectedCreatedNew = true; using Semaphore semaphore1 = CreateAndVerifySemaphore( @@ -70,7 +70,7 @@ public void Semaphore_Create_NameMultipleNew(string name) public void Semaphore_Create_CreateNewExisting() { string name = GetRandomName(); - var security = GetBasicSemaphoreSecurity(); + SemaphoreSecurity security = GetBasicSemaphoreSecurity(); using Semaphore SemaphoreNew = CreateAndVerifySemaphore( DefaultInitialCount, @@ -129,7 +129,7 @@ public void Semaphore_Create_BeyondMaxPathLength() [InlineData(SemaphoreRights.Modify | SemaphoreRights.Synchronize, AccessControlType.Deny)] public void Semaphore_Create_SpecificSecurity(SemaphoreRights rights, AccessControlType accessControl) { - var security = GetSemaphoreSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); + SemaphoreSecurity security = GetSemaphoreSecurity(WellKnownSidType.BuiltinUsersSid, rights, accessControl); CreateAndVerifySemaphore( DefaultInitialCount, @@ -150,9 +150,9 @@ private SemaphoreSecurity GetBasicSemaphoreSecurity() private SemaphoreSecurity GetSemaphoreSecurity(WellKnownSidType sid, SemaphoreRights rights, AccessControlType accessControl) { - var security = new SemaphoreSecurity(); + SemaphoreSecurity security = new SemaphoreSecurity(); SecurityIdentifier identity = new SecurityIdentifier(sid, null); - var accessRule = new SemaphoreAccessRule(identity, rights, accessControl); + SemaphoreAccessRule accessRule = new SemaphoreAccessRule(identity, rights, accessControl); security.AddAccessRule(accessRule); return security; }