diff --git a/src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs b/src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs index 21cc3dc01069..74b5dacf9778 100644 --- a/src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs +++ b/src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs @@ -48,11 +48,22 @@ public void EnsureCapacity(int capacity) Grow(capacity - _chars.Length); } + /// + /// Get a pinnable reference to the builder. + /// Does not ensure there is a null char after + /// This overload is pattern matched in the C# 7.3+ compiler so you can omit + /// the explicit method call, and write eg "fixed (char* c = builder)" + /// + public ref char GetPinnableReference() + { + return ref MemoryMarshal.GetReference(_chars); + } + /// /// Get a pinnable reference to the builder. /// /// Ensures that the builder has a null char after - public ref char GetPinnableReference(bool terminate = false) + public ref char GetPinnableReference(bool terminate) { if (terminate) { diff --git a/src/Common/src/Interop/Windows/Interop.Errors.cs b/src/Common/src/Interop/Windows/Interop.Errors.cs index 7836b049eb87..71e60528821d 100644 --- a/src/Common/src/Interop/Windows/Interop.Errors.cs +++ b/src/Common/src/Interop/Windows/Interop.Errors.cs @@ -57,6 +57,7 @@ internal partial class Errors internal const int ERROR_IO_INCOMPLETE = 0x3E4; internal const int ERROR_IO_PENDING = 0x3E5; internal const int ERROR_NO_TOKEN = 0x3f0; + internal const int ERROR_SERVICE_DOES_NOT_EXIST = 0x424; internal const int ERROR_DLL_INIT_FAILED = 0x45A; internal const int ERROR_COUNTER_TIMEOUT = 0x461; internal const int ERROR_NO_ASSOCIATION = 0x483; diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.ChangeServiceConfig2.cs b/src/Common/src/Interop/Windows/advapi32/Interop.ChangeServiceConfig2.cs index 9b92147954c6..42891bac1e08 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.ChangeServiceConfig2.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.ChangeServiceConfig2.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,9 +11,9 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] - public static extern bool ChangeServiceConfig2(IntPtr serviceHandle, uint infoLevel, ref SERVICE_DESCRIPTION serviceDesc); + public static extern bool ChangeServiceConfig2(SafeServiceHandle serviceHandle, uint infoLevel, ref SERVICE_DESCRIPTION serviceDesc); [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] - public static extern bool ChangeServiceConfig2(IntPtr serviceHandle, uint infoLevel, ref SERVICE_DELAYED_AUTOSTART_INFO serviceDesc); + public static extern bool ChangeServiceConfig2(SafeServiceHandle serviceHandle, uint infoLevel, ref SERVICE_DELAYED_AUTOSTART_INFO serviceDesc); } } diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.ControlService.cs b/src/Common/src/Interop/Windows/advapi32/Interop.ControlService.cs index 94cda1257b89..a4c345bb6c2d 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.ControlService.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.ControlService.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,7 +11,7 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] - internal extern unsafe static bool ControlService(IntPtr serviceHandle, int control, SERVICE_STATUS* pStatus); + internal extern unsafe static bool ControlService(SafeServiceHandle serviceHandle, int control, SERVICE_STATUS* pStatus); } } diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.CreateService.cs b/src/Common/src/Interop/Windows/advapi32/Interop.CreateService.cs index 9bb9a7367871..c56d55403d1d 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.CreateService.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.CreateService.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,7 +11,7 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] - public extern static IntPtr CreateService(IntPtr databaseHandle, string serviceName, string displayName, int access, int serviceType, + public extern static IntPtr CreateService(SafeServiceHandle databaseHandle, string serviceName, string displayName, int access, int serviceType, int startType, int errorControl, string binaryPath, string loadOrderGroup, IntPtr pTagId, string dependencies, string servicesStartName, string password); diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.DeleteService.cs b/src/Common/src/Interop/Windows/advapi32/Interop.DeleteService.cs index 4bd1b9801b00..705e3a95f909 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.DeleteService.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.DeleteService.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,6 +11,6 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] - public extern static bool DeleteService(IntPtr serviceHandle); + public extern static bool DeleteService(SafeServiceHandle serviceHandle); } } diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.EnumDependentServices.cs b/src/Common/src/Interop/Windows/advapi32/Interop.EnumDependentServices.cs index 878dc4f9766c..67a1cf76d069 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.EnumDependentServices.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.EnumDependentServices.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -11,7 +12,7 @@ internal partial class Advapi32 { [DllImport(Libraries.Advapi32, EntryPoint = "EnumDependentServicesW", CharSet = CharSet.Unicode, SetLastError = true)] internal extern static bool EnumDependentServices( - IntPtr serviceHandle, + SafeServiceHandle serviceHandle, int serviceState, IntPtr bufferOfENUM_SERVICE_STATUS, int bufSize, diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.EnumServicesStatusEx.cs b/src/Common/src/Interop/Windows/advapi32/Interop.EnumServicesStatusEx.cs index 84c79881f306..346adb3c8e0f 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.EnumServicesStatusEx.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.EnumServicesStatusEx.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -11,7 +12,7 @@ internal partial class Advapi32 { [DllImport(Libraries.Advapi32, EntryPoint = "EnumServicesStatusExW", CharSet = CharSet.Unicode, SetLastError = true)] internal extern static bool EnumServicesStatusEx( - IntPtr databaseHandle, + SafeServiceHandle databaseHandle, int infolevel, int serviceType, int serviceState, diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.GetServiceDisplayName.cs b/src/Common/src/Interop/Windows/advapi32/Interop.GetServiceDisplayName.cs new file mode 100644 index 000000000000..c2937fb037a8 --- /dev/null +++ b/src/Common/src/Interop/Windows/advapi32/Interop.GetServiceDisplayName.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 Microsoft.Win32.SafeHandles; +using System; +using System.Buffers; +using System.ComponentModel; +using System.Runtime.InteropServices; +using System.Text; + +internal partial class Interop +{ + internal partial class Advapi32 + { + [DllImport(Libraries.Advapi32, EntryPoint = "GetServiceDisplayNameW", CharSet = System.Runtime.InteropServices.CharSet.Unicode, SetLastError = true)] + internal static extern unsafe bool GetServiceDisplayName(SafeServiceHandle SCMHandle, string serviceName, char* displayName, ref int displayNameLength); + } +} diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.GetServiceKeyName.cs b/src/Common/src/Interop/Windows/advapi32/Interop.GetServiceKeyName.cs new file mode 100644 index 000000000000..de5256f14234 --- /dev/null +++ b/src/Common/src/Interop/Windows/advapi32/Interop.GetServiceKeyName.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 Microsoft.Win32.SafeHandles; +using System; +using System.Buffers; +using System.ComponentModel; +using System.Runtime.InteropServices; +using System.Text; + +internal partial class Interop +{ + internal partial class Advapi32 + { + [DllImport(Libraries.Advapi32, EntryPoint = "GetServiceKeyNameW", CharSet = System.Runtime.InteropServices.CharSet.Unicode, SetLastError = true)] + internal static extern unsafe bool GetServiceKeyName(SafeServiceHandle SCMHandle, string displayName, char* KeyName, ref int KeyNameLength); + } +} diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.OpenService.cs b/src/Common/src/Interop/Windows/advapi32/Interop.OpenService.cs index 5ef71c15e91d..813c322b61cc 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.OpenService.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.OpenService.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,6 +11,6 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, EntryPoint = "OpenServiceW", CharSet = CharSet.Unicode, SetLastError = true)] - internal extern static IntPtr OpenService(IntPtr databaseHandle, string serviceName, int access); + internal extern static IntPtr OpenService(SafeServiceHandle databaseHandle, string serviceName, int access); } } diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceConfig.cs b/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceConfig.cs index 73bfe711ffa5..c91db2fde1be 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceConfig.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceConfig.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,7 +11,7 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, EntryPoint = "QueryServiceConfigW", CharSet = CharSet.Unicode, SetLastError = true)] - internal extern static bool QueryServiceConfig(IntPtr serviceHandle, IntPtr queryServiceConfigPtr, int bufferSize, out int bytesNeeded); + internal extern static bool QueryServiceConfig(SafeServiceHandle serviceHandle, IntPtr queryServiceConfigPtr, int bufferSize, out int bytesNeeded); } } diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceStatus.cs b/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceStatus.cs index a271f7b3bfa2..6badc4cf1651 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceStatus.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.QueryServiceStatus.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,7 +11,7 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] - internal static extern unsafe bool QueryServiceStatus(IntPtr serviceHandle, SERVICE_STATUS* pStatus); + internal static extern unsafe bool QueryServiceStatus(SafeServiceHandle serviceHandle, SERVICE_STATUS* pStatus); } } diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.StartService.cs b/src/Common/src/Interop/Windows/advapi32/Interop.StartService.cs index cf29770da4ca..0695320bdd44 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.StartService.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.StartService.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. +using Microsoft.Win32.SafeHandles; using System; using System.Runtime.InteropServices; @@ -10,6 +11,6 @@ internal partial class Interop internal partial class Advapi32 { [DllImport(Libraries.Advapi32, EntryPoint = "StartServiceW", CharSet = CharSet.Unicode, SetLastError = true)] - internal extern static bool StartService(IntPtr serviceHandle, int argNum, IntPtr argPtrs); + internal extern static bool StartService(SafeServiceHandle serviceHandle, int argNum, IntPtr argPtrs); } } diff --git a/src/System.ServiceProcess.ServiceController/src/Microsoft/Win32/SafeHandles/SafeServiceHandle.cs b/src/System.ServiceProcess.ServiceController/src/Microsoft/Win32/SafeHandles/SafeServiceHandle.cs index 3cba4646193b..e8ed13dfe25a 100644 --- a/src/System.ServiceProcess.ServiceController/src/Microsoft/Win32/SafeHandles/SafeServiceHandle.cs +++ b/src/System.ServiceProcess.ServiceController/src/Microsoft/Win32/SafeHandles/SafeServiceHandle.cs @@ -9,6 +9,9 @@ namespace Microsoft.Win32.SafeHandles { + /// + /// Used to wrap handles gotten from OpenSCManager or OpenService + /// internal class SafeServiceHandle : SafeHandle { internal SafeServiceHandle(IntPtr handle) : base(IntPtr.Zero, true) diff --git a/src/System.ServiceProcess.ServiceController/src/Resources/Strings.resx b/src/System.ServiceProcess.ServiceController/src/Resources/Strings.resx index 2f40d0e9d42d..b8705287e84b 100644 --- a/src/System.ServiceProcess.ServiceController/src/Resources/Strings.resx +++ b/src/System.ServiceProcess.ServiceController/src/Resources/Strings.resx @@ -62,10 +62,10 @@ Arguments within the 'args' array passed to Start cannot be null. - MachineName value {0} is invalid. + MachineName '{0}' is invalid. - Cannot start service {0} on computer '{1}'. + Cannot start service '{0}' on computer '{1}'. The value of argument '{0}' ({1}) is invalid for Enum type '{2}'. @@ -77,22 +77,22 @@ MachineName was not set. - Service {0} was not found on computer '{1}'. + Service '{0}' was not found on computer '{1}'. Cannot open Service Control Manager on computer '{0}'. This operation might require other privileges. - Cannot open {0} service on computer '{1}'. + Cannot open '{0}' service on computer '{1}'. - Cannot pause {0} service on computer '{1}'. + Cannot pause '{0}' service on computer '{1}'. - Cannot resume {0} service on computer '{1}'. + Cannot resume '{0}' service on computer '{1}'. - Cannot stop {0} service on computer '{1}'. + Cannot stop '{0}' service on computer '{1}'. Time out has expired and the operation has not been completed. @@ -107,7 +107,7 @@ Cannot change service name when the service is running. - Service name {0} contains invalid characters, is empty, or is too long (max length = {1}). + Service name '{0}' contains invalid characters, is empty, or is too long (max length = {1}). Service has not been supplied. At least one object derived from ServiceBase is required in order to run. @@ -179,18 +179,18 @@ Failed in handling the PowerEvent. The error that occurred was: {0}. - Service {0} has been successfully installed. + Service '{0}' has been successfully installed. - Attempt to stop service {0}. + Attempt to stop service '{0}'. - Service {0} is being removed from the system... + Service '{0}' is being removed from the system... - Service {0} was successfully removed from the system. + Service '{0}' was successfully removed from the system. - Cannot control {0} service on computer '{1}'. + Cannot control '{0}' service on computer '{1}'. diff --git a/src/System.ServiceProcess.ServiceController/src/System.ServiceProcess.ServiceController.csproj b/src/System.ServiceProcess.ServiceController/src/System.ServiceProcess.ServiceController.csproj index e2d6f56fd352..caf54b745db6 100644 --- a/src/System.ServiceProcess.ServiceController/src/System.ServiceProcess.ServiceController.csproj +++ b/src/System.ServiceProcess.ServiceController/src/System.ServiceProcess.ServiceController.csproj @@ -12,6 +12,9 @@ net461-Windows_NT-Debug;net461-Windows_NT-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release;netstandard-Debug;netstandard-Release;netstandard-Windows_NT-Debug;netstandard-Windows_NT-Release + + Common\CoreLib\System\Text\ValueStringBuilder.cs + Common\Interop\Windows\Interop.Libraries.cs @@ -33,6 +36,12 @@ Common\Interop\Windows\Interop.EnumServicesStatusEx.cs + + Common\Interop\Windows\Interop.GetServiceDisplayName.cs + + + Common\Interop\Windows\Interop.GetServiceKeyName.cs + Common\Interop\Windows\Interop.OpenSCManager.cs @@ -93,13 +102,16 @@ + + + diff --git a/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs b/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs index e66ad3eba991..0f0b680460eb 100644 --- a/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs +++ b/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceBase.cs @@ -602,58 +602,65 @@ public static void Run(ServiceBase[] services) int sizeOfSERVICE_TABLE_ENTRY = Marshal.SizeOf(); IntPtr entriesPointer = Marshal.AllocHGlobal(checked((services.Length + 1) * sizeOfSERVICE_TABLE_ENTRY)); - SERVICE_TABLE_ENTRY[] entries = new SERVICE_TABLE_ENTRY[services.Length]; - bool multipleServices = services.Length > 1; - IntPtr structPtr; - - for (int index = 0; index < services.Length; ++index) + try { - services[index].Initialize(multipleServices); - entries[index] = services[index].GetEntry(); - structPtr = entriesPointer + sizeOfSERVICE_TABLE_ENTRY * index; - Marshal.StructureToPtr(entries[index], structPtr, fDeleteOld: false); - } + SERVICE_TABLE_ENTRY[] entries = new SERVICE_TABLE_ENTRY[services.Length]; + bool multipleServices = services.Length > 1; + IntPtr structPtr; - SERVICE_TABLE_ENTRY lastEntry = new SERVICE_TABLE_ENTRY(); + for (int index = 0; index < services.Length; ++index) + { + services[index].Initialize(multipleServices); + entries[index] = services[index].GetEntry(); + structPtr = entriesPointer + sizeOfSERVICE_TABLE_ENTRY * index; + Marshal.StructureToPtr(entries[index], structPtr, fDeleteOld: false); + } - lastEntry.callback = null; - lastEntry.name = (IntPtr)0; - structPtr = entriesPointer + sizeOfSERVICE_TABLE_ENTRY * services.Length; - Marshal.StructureToPtr(lastEntry, structPtr, fDeleteOld: false); + SERVICE_TABLE_ENTRY lastEntry = new SERVICE_TABLE_ENTRY(); - // While the service is running, this function will never return. It will return when the service - // is stopped. - // After it returns, SCM might terminate the process at any time - // (so subsequent code is not guaranteed to run). - bool res = StartServiceCtrlDispatcher(entriesPointer); + lastEntry.callback = null; + lastEntry.name = (IntPtr)0; + structPtr = entriesPointer + sizeOfSERVICE_TABLE_ENTRY * services.Length; + Marshal.StructureToPtr(lastEntry, structPtr, fDeleteOld: false); - foreach (ServiceBase service in services) - { - if (service._startFailedException != null) + // While the service is running, this function will never return. It will return when the service + // is stopped. + // After it returns, SCM might terminate the process at any time + // (so subsequent code is not guaranteed to run). + bool res = StartServiceCtrlDispatcher(entriesPointer); + + foreach (ServiceBase service in services) { - // Propagate exceptions throw during OnStart. - // Note that this same exception is also thrown from ServiceMainCallback - // (so SCM can see it as well). - service._startFailedException.Throw(); + if (service._startFailedException != null) + { + // Propagate exceptions throw during OnStart. + // Note that this same exception is also thrown from ServiceMainCallback + // (so SCM can see it as well). + service._startFailedException.Throw(); + } } - } - string errorMessage = ""; + string errorMessage = ""; - if (!res) - { - errorMessage = new Win32Exception().Message; - Console.WriteLine(SR.CantStartFromCommandLine); - } - - foreach (ServiceBase service in services) - { - service.Dispose(); if (!res) { - service.WriteLogEntry(SR.Format(SR.StartFailed, errorMessage), true); + errorMessage = new Win32Exception().Message; + Console.WriteLine(SR.CantStartFromCommandLine); + } + + foreach (ServiceBase service in services) + { + service.Dispose(); + if (!res) + { + service.WriteLogEntry(SR.Format(SR.StartFailed, errorMessage), true); + } } } + finally + { + Marshal.FreeHGlobal(entriesPointer); + } } /// diff --git a/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs b/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs index 0dabc4e9cc64..35123d0ea4ed 100644 --- a/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs +++ b/src/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs @@ -19,13 +19,14 @@ namespace System.ServiceProcess /// and manipulate it or get information about it. public class ServiceController : Component { - private string _machineName; + private string _machineName; // Never null private readonly ManualResetEvent _waitForStatusSignal = new ManualResetEvent(false); private const string DefaultMachineName = "."; private string _name; private string _eitherName; private string _displayName; + private int _commandsAccepted; private bool _statusGenerated; private bool _startTypeInitialized; @@ -37,11 +38,9 @@ public class ServiceController : Component private ServiceController[] _servicesDependedOn; private ServiceStartMode _startType; - private const int SERVICENAMEMAXLENGTH = 80; - private const int DISPLAYNAMEBUFFERSIZE = 256; - public ServiceController() { + _machineName = DefaultMachineName; _type = Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_ALL; } @@ -133,7 +132,7 @@ public string DisplayName { get { - if (_displayName == null) + if (String.IsNullOrEmpty(_displayName)) GenerateNames(); return _displayName; } @@ -163,8 +162,7 @@ public ServiceController[] DependentServices { if (_dependentServices == null) { - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_ENUMERATE_DEPENDENTS); - try + using (var serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_ENUMERATE_DEPENDENTS)) { // figure out how big a buffer we need to get the info int bytesNeeded = 0; @@ -207,10 +205,6 @@ public ServiceController[] DependentServices Marshal.FreeHGlobal(enumBuffer); } } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } return _dependentServices; @@ -247,7 +241,7 @@ public string ServiceName { get { - if (_name == null) + if (String.IsNullOrEmpty(_name)) GenerateNames(); return _name; } @@ -280,8 +274,7 @@ public unsafe ServiceController[] ServicesDependedOn if (_servicesDependedOn != null) return _servicesDependedOn; - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_QUERY_CONFIG); - try + using (var serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_QUERY_CONFIG)) { int bytesNeeded = 0; bool success = Interop.Advapi32.QueryServiceConfig(serviceHandle, IntPtr.Zero, 0, out bytesNeeded); @@ -358,10 +351,6 @@ public unsafe ServiceController[] ServicesDependedOn Marshal.FreeHGlobal(bufPtr); } } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } } @@ -372,11 +361,8 @@ public ServiceStartMode StartType if (_startTypeInitialized) return _startType; - IntPtr serviceHandle = IntPtr.Zero; - try + using (var serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_QUERY_CONFIG)) { - serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_QUERY_CONFIG); - int bytesNeeded = 0; bool success = Interop.Advapi32.QueryServiceConfig(serviceHandle, IntPtr.Zero, 0, out bytesNeeded); @@ -385,10 +371,9 @@ public ServiceStartMode StartType throw new Win32Exception(lastError); // get the info - IntPtr bufPtr = IntPtr.Zero; + IntPtr bufPtr = Marshal.AllocHGlobal((IntPtr)bytesNeeded); try { - bufPtr = Marshal.AllocHGlobal((IntPtr)bytesNeeded); success = Interop.Advapi32.QueryServiceConfig(serviceHandle, bufPtr, bytesNeeded, out bytesNeeded); if (!success) throw new Win32Exception(Marshal.GetLastWin32Error()); @@ -401,14 +386,8 @@ public ServiceStartMode StartType } finally { - if (bufPtr != IntPtr.Zero) - Marshal.FreeHGlobal(bufPtr); - } - } - finally - { - if (serviceHandle != IntPtr.Zero) - Interop.Advapi32.CloseServiceHandle(serviceHandle); + Marshal.FreeHGlobal(bufPtr); + } } return _startType; @@ -419,7 +398,7 @@ public SafeHandle ServiceHandle { get { - return new SafeServiceHandle(GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_ALL_ACCESS)); + return GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_ALL_ACCESS); } } @@ -448,13 +427,14 @@ private static bool CheckMachineName(string value) return !string.IsNullOrWhiteSpace(value) && value.IndexOf('\\') == -1; } + /// + /// Closes the handle to the service manager, but does not + /// mark the class as disposed. + /// + /// + /// Violates design guidelines by not matching Dispose() -- matches .NET Framework + /// public void Close() - { - Dispose(); - } - - /// Disconnects this object from the service and frees any allocated resources. - protected override void Dispose(bool disposing) { if (_serviceManagerHandle != null) { @@ -465,15 +445,23 @@ protected override void Dispose(bool disposing) _statusGenerated = false; _startTypeInitialized = false; _type = Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_ALL; + } + + /// + /// Closes the handle to the service manager, and disposes. + /// + protected override void Dispose(bool disposing) + { + Close(); _disposed = true; + base.Dispose(disposing); } private unsafe void GenerateStatus() { if (!_statusGenerated) { - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_QUERY_STATUS); - try + using (var serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_QUERY_STATUS)) { Interop.Advapi32.SERVICE_STATUS svcStatus = new Interop.Advapi32.SERVICE_STATUS(); bool success = Interop.Advapi32.QueryServiceStatus(serviceHandle, &svcStatus); @@ -485,107 +473,139 @@ private unsafe void GenerateStatus() _type = svcStatus.serviceType; _statusGenerated = true; } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } } private unsafe void GenerateNames() { - if (_machineName.Length == 0) - throw new ArgumentException(SR.NoMachineName); - - IntPtr databaseHandle = IntPtr.Zero; - IntPtr memory = IntPtr.Zero; - int bytesNeeded; - int servicesReturned; - int resumeHandle = 0; + GetDataBaseHandleWithConnectAccess(); - try + if (String.IsNullOrEmpty(_name)) { - databaseHandle = GetDataBaseHandleWithEnumerateAccess(_machineName); - Interop.Advapi32.EnumServicesStatusEx( - databaseHandle, - Interop.Advapi32.ServiceControllerOptions.SC_ENUM_PROCESS_INFO, - Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_WIN32 | Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_DRIVER, - Interop.Advapi32.StatusOptions.STATUS_ALL, - IntPtr.Zero, - 0, - out bytesNeeded, - out servicesReturned, - ref resumeHandle, - null); + // Figure out the _name based on the information we have. + // We must either have _displayName or the constructor parameter _eitherName. + string userGivenName = String.IsNullOrEmpty(_eitherName) ? _displayName : _eitherName; - memory = Marshal.AllocHGlobal(bytesNeeded); + if (String.IsNullOrEmpty(userGivenName)) + throw new InvalidOperationException(SR.Format(SR.ServiceName, userGivenName, ServiceBase.MaxNameLength.ToString(CultureInfo.CurrentCulture))); - Interop.Advapi32.EnumServicesStatusEx( - databaseHandle, - Interop.Advapi32.ServiceControllerOptions.SC_ENUM_PROCESS_INFO, - Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_WIN32 | Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_DRIVER, - Interop.Advapi32.StatusOptions.STATUS_ALL, - memory, - bytesNeeded, - out bytesNeeded, - out servicesReturned, - ref resumeHandle, - null); - - // Since the service name of one service cannot be equal to the - // service or display name of another service, we can safely - // loop through all services checking if either the service or - // display name matches the user given name. If there is a - // match, then we've found the service. - for (int i = 0; i < servicesReturned; i++) + // Try it as a display name + string result = GetServiceKeyName(_serviceManagerHandle, userGivenName); + + if (result != null) { - IntPtr structPtr = (IntPtr)((long)memory + (i * Marshal.SizeOf())); - Interop.Advapi32.ENUM_SERVICE_STATUS_PROCESS status = new Interop.Advapi32.ENUM_SERVICE_STATUS_PROCESS(); - Marshal.PtrToStructure(structPtr, status); + // Now we have both + _name = result; + _displayName = userGivenName; + _eitherName = null; + return; + } - if (string.Equals(_eitherName, status.serviceName, StringComparison.OrdinalIgnoreCase) || - string.Equals(_eitherName, status.displayName, StringComparison.OrdinalIgnoreCase)) - { - if (_name == null) - { - _name = status.serviceName; - } + // Try it as a service name + result = GetServiceDisplayName(_serviceManagerHandle, userGivenName); - if (_displayName == null) - { - _displayName = status.displayName; - } + if (result == null) + { + throw new InvalidOperationException(SR.Format(SR.NoService, userGivenName, _machineName), new Win32Exception(Interop.Errors.ERROR_SERVICE_DOES_NOT_EXIST)); + } - _eitherName = string.Empty; - return; - } + _name = userGivenName; + _displayName = result; + _eitherName = null; + } + else if (String.IsNullOrEmpty(_displayName)) + { + // We must have _name + string result = GetServiceDisplayName(_serviceManagerHandle, _name); + + if (result == null) + { + throw new InvalidOperationException(SR.Format(SR.NoService, _name, _machineName), new Win32Exception(Interop.Errors.ERROR_SERVICE_DOES_NOT_EXIST)); } - throw new InvalidOperationException(SR.Format(SR.NoService, _eitherName, _machineName)); + _displayName = result; + _eitherName = null; } - finally + } + + /// + /// Gets service name (key name) from service display name. + /// Returns null if service is not found. + /// + private unsafe string GetServiceKeyName(SafeServiceHandle SCMHandle, string serviceDisplayName) + { + Span initialBuffer = stackalloc char[256]; + var builder = new ValueStringBuilder(initialBuffer); + int bufLen; + while (true) { - Marshal.FreeHGlobal(memory); - if (databaseHandle != IntPtr.Zero) + bufLen = builder.Capacity; + fixed (char* c = builder) + { + if (Interop.Advapi32.GetServiceKeyName(SCMHandle, serviceDisplayName, c, ref bufLen)) + break; + } + + int lastError = Marshal.GetLastWin32Error(); + if (lastError == Interop.Errors.ERROR_SERVICE_DOES_NOT_EXIST) + { + return null; + } + else if (lastError != Interop.Errors.ERROR_INSUFFICIENT_BUFFER) { - Interop.Advapi32.CloseServiceHandle(databaseHandle); + throw new InvalidOperationException(SR.Format(SR.NoService, serviceDisplayName, _machineName), new Win32Exception(lastError)); } + + builder.EnsureCapacity(bufLen + 1); // Does not include null } + + builder.Length = bufLen; + return builder.ToString(); } - private static IntPtr GetDataBaseHandleWithAccess(string machineName, int serviceControlManagerAccess) + private unsafe string GetServiceDisplayName(SafeServiceHandle SCMHandle, string serviceName) { - IntPtr databaseHandle = IntPtr.Zero; + var builder = new ValueStringBuilder(4096); + int bufLen; + while (true) + { + bufLen = builder.Capacity; + fixed (char* c = builder) + { + if (Interop.Advapi32.GetServiceDisplayName(SCMHandle, serviceName, c, ref bufLen)) + break; + } + + int lastError = Marshal.GetLastWin32Error(); + if (lastError == Interop.Errors.ERROR_SERVICE_DOES_NOT_EXIST) + { + return null; + } + else if (lastError != Interop.Errors.ERROR_INSUFFICIENT_BUFFER) + { + throw new InvalidOperationException(SR.Format(SR.NoService, serviceName, _machineName), new Win32Exception(lastError)); + } + + builder.EnsureCapacity(bufLen + 1); // Does not include null + } + + builder.Length = bufLen; + return builder.ToString(); + } + + private static SafeServiceHandle GetDataBaseHandleWithAccess(string machineName, int serviceControlManagerAccess) + { + SafeServiceHandle databaseHandle = null; if (machineName.Equals(DefaultMachineName) || machineName.Length == 0) { - databaseHandle = Interop.Advapi32.OpenSCManager(null, null, serviceControlManagerAccess); + databaseHandle = new SafeServiceHandle(Interop.Advapi32.OpenSCManager(null, null, serviceControlManagerAccess)); } else { - databaseHandle = Interop.Advapi32.OpenSCManager(machineName, null, serviceControlManagerAccess); + databaseHandle = new SafeServiceHandle(Interop.Advapi32.OpenSCManager(machineName, null, serviceControlManagerAccess)); } - if (databaseHandle == IntPtr.Zero) + if (databaseHandle.IsInvalid) { Exception inner = new Win32Exception(Marshal.GetLastWin32Error()); throw new InvalidOperationException(SR.Format(SR.OpenSC, machineName), inner); @@ -604,15 +624,10 @@ private void GetDataBaseHandleWithConnectAccess() // get a handle to SCM with connect access and store it in serviceManagerHandle field. if (_serviceManagerHandle == null) { - _serviceManagerHandle = new SafeServiceHandle(GetDataBaseHandleWithAccess(_machineName, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_CONNECT)); + _serviceManagerHandle = GetDataBaseHandleWithAccess(_machineName, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_CONNECT); } } - private static IntPtr GetDataBaseHandleWithEnumerateAccess(string machineName) - { - return GetDataBaseHandleWithAccess(machineName, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_ENUMERATE_SERVICE); - } - /// Gets all the device-driver services on the local machine. public static ServiceController[] GetDevices() { @@ -625,14 +640,13 @@ public static ServiceController[] GetDevices(string machineName) return GetServicesOfType(machineName, Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_DRIVER); } - /// Opens a handle for the current service. The handle must be closed with - /// a call to Interop.CloseServiceHandle(). - private IntPtr GetServiceHandle(int desiredAccess) + /// Opens a handle for the current service. The handle must be Dispose()'d. + private SafeServiceHandle GetServiceHandle(int desiredAccess) { GetDataBaseHandleWithConnectAccess(); - IntPtr serviceHandle = Interop.Advapi32.OpenService(_serviceManagerHandle.DangerousGetHandle(), ServiceName, desiredAccess); - if (serviceHandle == IntPtr.Zero) + var serviceHandle = new SafeServiceHandle(Interop.Advapi32.OpenService(_serviceManagerHandle, ServiceName, desiredAccess)); + if (serviceHandle.IsInvalid) { Exception inner = new Win32Exception(Marshal.GetLastWin32Error()); throw new InvalidOperationException(SR.Format(SR.OpenService, ServiceName, _machineName), inner); @@ -671,17 +685,14 @@ private static ServiceController[] GetServicesOfType(string machineName, int ser /// Helper for GetDevices, GetServices, and ServicesDependedOn private static T[] GetServices(string machineName, int serviceType, string group, Func selector) { - IntPtr databaseHandle = IntPtr.Zero; - IntPtr memory = IntPtr.Zero; int bytesNeeded; int servicesReturned; int resumeHandle = 0; T[] services; - try + using (SafeServiceHandle databaseHandle = GetDataBaseHandleWithAccess(machineName, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_ENUMERATE_SERVICE)) { - databaseHandle = GetDataBaseHandleWithEnumerateAccess(machineName); Interop.Advapi32.EnumServicesStatusEx( databaseHandle, Interop.Advapi32.ServiceControllerOptions.SC_ENUM_PROCESS_INFO, @@ -694,42 +705,39 @@ private static T[] GetServices(string machineName, int serviceType, string gr ref resumeHandle, group); - memory = Marshal.AllocHGlobal((IntPtr)bytesNeeded); - - // - // Get the set of services - // - Interop.Advapi32.EnumServicesStatusEx( - databaseHandle, - Interop.Advapi32.ServiceControllerOptions.SC_ENUM_PROCESS_INFO, - serviceType, - Interop.Advapi32.StatusOptions.STATUS_ALL, - memory, - bytesNeeded, - out bytesNeeded, - out servicesReturned, - ref resumeHandle, - group); - - // - // Go through the block of memory it returned to us and select the results - // - services = new T[servicesReturned]; - for (int i = 0; i < servicesReturned; i++) + IntPtr memory = Marshal.AllocHGlobal((IntPtr)bytesNeeded); + try { - IntPtr structPtr = (IntPtr)((long)memory + (i * Marshal.SizeOf())); - Interop.Advapi32.ENUM_SERVICE_STATUS_PROCESS status = new Interop.Advapi32.ENUM_SERVICE_STATUS_PROCESS(); - Marshal.PtrToStructure(structPtr, status); - services[i] = selector(status); + // + // Get the set of services + // + Interop.Advapi32.EnumServicesStatusEx( + databaseHandle, + Interop.Advapi32.ServiceControllerOptions.SC_ENUM_PROCESS_INFO, + serviceType, + Interop.Advapi32.StatusOptions.STATUS_ALL, + memory, + bytesNeeded, + out bytesNeeded, + out servicesReturned, + ref resumeHandle, + group); + + // + // Go through the block of memory it returned to us and select the results + // + services = new T[servicesReturned]; + for (int i = 0; i < servicesReturned; i++) + { + IntPtr structPtr = (IntPtr)((long)memory + (i * Marshal.SizeOf())); + Interop.Advapi32.ENUM_SERVICE_STATUS_PROCESS status = new Interop.Advapi32.ENUM_SERVICE_STATUS_PROCESS(); + Marshal.PtrToStructure(structPtr, status); + services[i] = selector(status); + } } - } - finally - { - Marshal.FreeHGlobal(memory); - - if (databaseHandle != IntPtr.Zero) + finally { - Interop.Advapi32.CloseServiceHandle(databaseHandle); + Marshal.FreeHGlobal(memory); } } @@ -739,8 +747,7 @@ private static T[] GetServices(string machineName, int serviceType, string gr /// Suspends a service's operation. public unsafe void Pause() { - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_PAUSE_CONTINUE); - try + using (var serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_PAUSE_CONTINUE)) { Interop.Advapi32.SERVICE_STATUS status = new Interop.Advapi32.SERVICE_STATUS(); bool result = Interop.Advapi32.ControlService(serviceHandle, Interop.Advapi32.ControlOptions.CONTROL_PAUSE, &status); @@ -750,17 +757,12 @@ public unsafe void Pause() throw new InvalidOperationException(SR.Format(SR.PauseService, ServiceName, _machineName), inner); } } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } /// Continues a service after it has been paused. public unsafe void Continue() { - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_PAUSE_CONTINUE); - try + using (var serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_PAUSE_CONTINUE)) { Interop.Advapi32.SERVICE_STATUS status = new Interop.Advapi32.SERVICE_STATUS(); bool result = Interop.Advapi32.ControlService(serviceHandle, Interop.Advapi32.ControlOptions.CONTROL_CONTINUE, &status); @@ -770,16 +772,11 @@ public unsafe void Continue() throw new InvalidOperationException(SR.Format(SR.ResumeService, ServiceName, _machineName), inner); } } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } public unsafe void ExecuteCommand(int command) { - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_USER_DEFINED_CONTROL); - try + using (var serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_USER_DEFINED_CONTROL)) { Interop.Advapi32.SERVICE_STATUS status = new Interop.Advapi32.SERVICE_STATUS(); bool result = Interop.Advapi32.ControlService(serviceHandle, command, &status); @@ -789,10 +786,6 @@ public unsafe void ExecuteCommand(int command) throw new InvalidOperationException(SR.Format(SR.ControlService, ServiceName, MachineName), inner); } } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } /// Refreshes all property values. @@ -816,9 +809,7 @@ public void Start(string[] args) if (args == null) throw new ArgumentNullException(nameof(args)); - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_START); - - try + using (SafeServiceHandle serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_START)) { IntPtr[] argPtrs = new IntPtr[args.Length]; int i = 0; @@ -858,10 +849,6 @@ public void Start(string[] args) argPtrsHandle.Free(); } } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } /// Stops the service. If any other services depend on this one for operation, @@ -869,9 +856,7 @@ public void Start(string[] args) /// of services. public unsafe void Stop() { - IntPtr serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_STOP); - - try + using (SafeServiceHandle serviceHandle = GetServiceHandle(Interop.Advapi32.ServiceOptions.SERVICE_STOP)) { // Before stopping this service, stop all the dependent services that are running. // (It's OK not to cache the result of getting the DependentServices property because it caches on its own.) @@ -894,10 +879,6 @@ public unsafe void Stop() throw new InvalidOperationException(SR.Format(SR.StopService, ServiceName, _machineName), inner); } } - finally - { - Interop.Advapi32.CloseServiceHandle(serviceHandle); - } } /// Waits infinitely until the service has reached the given status. diff --git a/src/System.ServiceProcess.ServiceController/tests/SafeServiceControllerTests.cs b/src/System.ServiceProcess.ServiceController/tests/SafeServiceControllerTests.cs index 441f55c5622f..cf3a26ac4dd4 100644 --- a/src/System.ServiceProcess.ServiceController/tests/SafeServiceControllerTests.cs +++ b/src/System.ServiceProcess.ServiceController/tests/SafeServiceControllerTests.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. +using System.ComponentModel; using Xunit; namespace System.ServiceProcess.Tests @@ -59,6 +60,22 @@ public static void GetServices() Assert.True(foundOtherSvc, "foundOtherSvc"); } + [Theory] + [InlineData(null)] + [InlineData("")] + public static void ConstructWithBadServiceName(string value) + { + Assert.Throws(() => new ServiceController(value)); + } + + [Fact] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Full framework does not throw")] + public static void Initialize_GetNames() + { + Assert.Throws(() => new ServiceController().ServiceName); + Assert.Throws(() => new ServiceController().DisplayName); + } + [Fact] public static void GetDevices() { @@ -87,6 +104,96 @@ public static void EnumerateDeviceService() Assert.Equal(devices[0].MachineName, actual.MachineName); } + [Fact] + public static void NonExistentService_GetStatus() + { + var controller = new ServiceController(Guid.NewGuid().ToString("N")); + Exception exception = Assert.Throws(() => controller.Status); + Assert.IsType(exception.InnerException); + } + + [Fact] + public static void NonExistentService_GetDisplayName() + { + var controller = new ServiceController(Guid.NewGuid().ToString("N")); + Exception exception = Assert.Throws(() => controller.DisplayName); + Assert.IsType(exception.InnerException); + } + + [Fact] + public static void SetNameToNonexistentService_GetStatus() + { + var controller = new ServiceController(); + controller.ServiceName = Guid.NewGuid().ToString("N"); + Exception exception = Assert.Throws(() => controller.Status); + Assert.IsType(exception.InnerException); + } + + [Fact] + public static void SetNameToNonexistentService_GetDisplayName() + { + var controller = new ServiceController(); + controller.ServiceName = Guid.NewGuid().ToString("N"); + Exception exception = Assert.Throws(() => controller.DisplayName); + Assert.IsType(exception.InnerException); + } + + [Fact] + public static void SetDisplayNameToNonexistentService_GetStatus() + { + var controller = new ServiceController(); + controller.DisplayName = Guid.NewGuid().ToString("N"); + Exception exception = Assert.Throws(() => controller.Status); + Assert.IsType(exception.InnerException); + } + + [Fact] + public static void SetDisplayNameToNonexistentService_GetServiceName() + { + var controller = new ServiceController(); + controller.DisplayName = Guid.NewGuid().ToString("N"); + Exception exception = Assert.Throws(() => controller.ServiceName); + Assert.IsType(exception.InnerException); + } + + [Fact] + public static void SetServiceName_GetDisplayName() + { + var keyIsoDisplayName = new ServiceController(KeyIsoSvcName).DisplayName; + + var controller = new ServiceController(); + controller.ServiceName = KeyIsoSvcName; + Assert.Equal(keyIsoDisplayName, controller.DisplayName); + } + + [Fact] + public static void SetDisplayName_GetServiceName() + { + var keyIsoDisplayName = new ServiceController(KeyIsoSvcName).DisplayName; + + var controller = new ServiceController(); + controller.DisplayName = keyIsoDisplayName; + Assert.Equal(KeyIsoSvcName.ToLowerInvariant(), controller.ServiceName.ToLowerInvariant()); + } + + [Fact] + public static void InitializeServiceName_GetDisplayName() + { + var controller = new ServiceController(KeyIsoSvcName); + Assert.Equal(KeyIsoSvcName, controller.ServiceName); + Assert.NotEmpty(controller.DisplayName); + } + + [Fact] + public static void InitializeDisplayName_GetServiceName() + { + var keyIsoDisplayName = new ServiceController(KeyIsoSvcName).DisplayName; + + var controller = new ServiceController(keyIsoDisplayName); + Assert.Equal(KeyIsoSvcName.ToLowerInvariant(), controller.ServiceName.ToLowerInvariant()); + Assert.Equal(keyIsoDisplayName, controller.DisplayName); + } + [Fact] public static void WaitForStatusTimeout() { diff --git a/src/System.ServiceProcess.ServiceController/tests/ServiceControllerTests.cs b/src/System.ServiceProcess.ServiceController/tests/ServiceControllerTests.cs index c39395cb40d7..f56b382eac15 100644 --- a/src/System.ServiceProcess.ServiceController/tests/ServiceControllerTests.cs +++ b/src/System.ServiceProcess.ServiceController/tests/ServiceControllerTests.cs @@ -25,8 +25,7 @@ public ServiceControllerTests() private void AssertExpectedProperties(ServiceController testServiceController) { - var comparer = PlatformDetection.IsFullFramework ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal; // Full framework upper cases the name - Assert.Equal(_testService.TestServiceName, testServiceController.ServiceName, comparer); + Assert.Equal(_testService.TestServiceName, testServiceController.ServiceName, StringComparer.OrdinalIgnoreCase); Assert.Equal(_testService.TestServiceDisplayName, testServiceController.DisplayName); Assert.Equal(_testService.TestMachineName, testServiceController.MachineName); Assert.Equal(ServiceType.Win32OwnProcess, testServiceController.ServiceType); diff --git a/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/System.ServiceProcess.ServiceController.TestService.csproj b/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/System.ServiceProcess.ServiceController.TestService.csproj index 06d5a3c7ce6a..2b5d1d4d8fa3 100644 --- a/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/System.ServiceProcess.ServiceController.TestService.csproj +++ b/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/System.ServiceProcess.ServiceController.TestService.csproj @@ -12,6 +12,9 @@ Component + + Microsoft\Win32\SafeHandles\SafeServiceHandle.cs + Common\Interop\Windows\Interop.Libraries.cs diff --git a/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs b/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs index db9b3fe2eb7c..101d1bc71e3e 100644 --- a/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs +++ b/src/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.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. +using Microsoft.Win32.SafeHandles; using System.ComponentModel; using System.Diagnostics; using System.Text; @@ -52,7 +53,7 @@ public unsafe void Install() ServiceCommandLine = $"\"{processName}\" {arguments}"; } - //Build servicesDependedOn string + // Build servicesDependedOn string string servicesDependedOn = null; if (ServicesDependedOn.Length > 0) { @@ -70,53 +71,45 @@ public unsafe void Install() } // Open the service manager - IntPtr serviceManagerHandle = Interop.Advapi32.OpenSCManager(null, null, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_ALL); - IntPtr serviceHandle = IntPtr.Zero; - if (serviceManagerHandle == IntPtr.Zero) - throw new InvalidOperationException("Cannot open Service Control Manager"); - - try + using (var serviceManagerHandle = new SafeServiceHandle(Interop.Advapi32.OpenSCManager(null, null, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_ALL))) { + if (serviceManagerHandle.IsInvalid) + throw new InvalidOperationException("Cannot open Service Control Manager"); + // Install the service - serviceHandle = Interop.Advapi32.CreateService(serviceManagerHandle, ServiceName, + using (var serviceHandle = new SafeServiceHandle(Interop.Advapi32.CreateService(serviceManagerHandle, ServiceName, DisplayName, Interop.Advapi32.ServiceAccessOptions.ACCESS_TYPE_ALL, Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_WIN32_OWN_PROCESS, (int)StartType, Interop.Advapi32.ServiceStartErrorModes.ERROR_CONTROL_NORMAL, - ServiceCommandLine, null, IntPtr.Zero, servicesDependedOn, username, password); - - if (serviceHandle == IntPtr.Zero) - throw new Win32Exception("Cannot create service"); + ServiceCommandLine, null, IntPtr.Zero, servicesDependedOn, username, password))) + { + if (serviceHandle.IsInvalid) + throw new Win32Exception("Cannot create service"); - // A local variable in an unsafe method is already fixed -- so we don't need a "fixed { }" blocks to protect - // across the p/invoke calls below. + // A local variable in an unsafe method is already fixed -- so we don't need a "fixed { }" blocks to protect + // across the p/invoke calls below. - if (Description.Length != 0) - { - Interop.Advapi32.SERVICE_DESCRIPTION serviceDesc = new Interop.Advapi32.SERVICE_DESCRIPTION(); - serviceDesc.description = Marshal.StringToHGlobalUni(Description); - bool success = Interop.Advapi32.ChangeServiceConfig2(serviceHandle, Interop.Advapi32.ServiceConfigOptions.SERVICE_CONFIG_DESCRIPTION, ref serviceDesc); - Marshal.FreeHGlobal(serviceDesc.description); - if (!success) - throw new Win32Exception("Cannot set description"); - } + if (Description.Length != 0) + { + Interop.Advapi32.SERVICE_DESCRIPTION serviceDesc = new Interop.Advapi32.SERVICE_DESCRIPTION(); + serviceDesc.description = Marshal.StringToHGlobalUni(Description); + bool success = Interop.Advapi32.ChangeServiceConfig2(serviceHandle, Interop.Advapi32.ServiceConfigOptions.SERVICE_CONFIG_DESCRIPTION, ref serviceDesc); + Marshal.FreeHGlobal(serviceDesc.description); + if (!success) + throw new Win32Exception("Cannot set description"); + } - // Start the service after creating it - using (ServiceController svc = new ServiceController(ServiceName)) - { - if (svc.Status != ServiceControllerStatus.Running) + // Start the service after creating it + using (ServiceController svc = new ServiceController(ServiceName)) { - svc.Start(); - if (!ServiceName.StartsWith("PropagateExceptionFromOnStart")) - svc.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromSeconds(30)); + if (svc.Status != ServiceControllerStatus.Running) + { + svc.Start(); + if (!ServiceName.StartsWith("PropagateExceptionFromOnStart")) + svc.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromSeconds(30)); + } } } } - finally - { - if (serviceHandle != IntPtr.Zero) - Interop.Advapi32.CloseServiceHandle(serviceHandle); - - Interop.Advapi32.CloseServiceHandle(serviceManagerHandle); - } } public void RemoveService() @@ -170,28 +163,21 @@ private void StopService() private void DeleteService() { - IntPtr serviceManagerHandle = Interop.Advapi32.OpenSCManager(null, null, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_ALL); - if (serviceManagerHandle == IntPtr.Zero) - throw new Win32Exception("Could not open SCM"); - - IntPtr serviceHandle = IntPtr.Zero; - try + using (var serviceManagerHandle = new SafeServiceHandle(Interop.Advapi32.OpenSCManager(null, null, Interop.Advapi32.ServiceControllerOptions.SC_MANAGER_ALL))) { - serviceHandle = Interop.Advapi32.OpenService(serviceManagerHandle, - ServiceName, Interop.Advapi32.ServiceOptions.STANDARD_RIGHTS_DELETE); - - if (serviceHandle == IntPtr.Zero) - throw new Win32Exception($"Could not find service '{ServiceName}'"); + if (serviceManagerHandle.IsInvalid) + throw new Win32Exception("Could not open SCM"); - if (!Interop.Advapi32.DeleteService(serviceHandle)) - throw new Win32Exception($"Could not delete service '{ServiceName}'"); - } - finally - { - if (serviceHandle != IntPtr.Zero) - Interop.Advapi32.CloseServiceHandle(serviceHandle); + using (var serviceHandle = new SafeServiceHandle(Interop.Advapi32.OpenService(serviceManagerHandle, ServiceName, Interop.Advapi32.ServiceOptions.STANDARD_RIGHTS_DELETE))) + { + if (serviceHandle.IsInvalid) + throw new Win32Exception($"Could not find service '{ServiceName}'"); - Interop.Advapi32.CloseServiceHandle(serviceManagerHandle); + if (!Interop.Advapi32.DeleteService(serviceHandle)) + { + throw new Win32Exception($"Could not delete service '{ServiceName}'"); + } + } } } }