Fix ServiceController name population perf (dotnet/corefx#32072)

* Fix ServiceController name population perf * Split tests * Remove dead field * Remove new use of DangerousGetHandle * SafeHandle all the things! * VSB dotnet/corefx#1 * VSB dotnet/corefx#2 * Fix GLE * Initialize machineName in ctor * Test for empty name ex * Null names * Inadvertent edit * Unix build * Move interop into class * Reverse SafeHandle for HAllocGlobal * Fix tests * Disable test for NETFX * CR feedback * Pattern matching on VSB * Direct call * typo Commit migrated from dotnet/corefx@76c8587
picenka21 · Sep 4, 2018 · b8c2921 · b8c2921
1 parent 42c364a
commit b8c2921
Show file tree

Hide file tree

Showing 22 changed files with 460 additions and 313 deletions.
diff --git a/src/libraries/Common/src/Interop/Windows/Interop.Errors.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.ChangeServiceConfig2.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.ControlService.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.CreateService.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.DeleteService.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.EnumDependentServices.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.EnumServicesStatusEx.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.GetServiceDisplayName.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.GetServiceKeyName.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.OpenService.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.QueryServiceConfig.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.QueryServiceStatus.cs b/src/libraries/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/libraries/Common/src/Interop/Windows/advapi32/Interop.StartService.cs b/src/libraries/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/...tem.ServiceProcess.ServiceController/src/Microsoft/Win32/SafeHandles/SafeServiceHandle.cs b/...tem.ServiceProcess.ServiceController/src/Microsoft/Win32/SafeHandles/SafeServiceHandle.cs
@@ -9,6 +9,9 @@
 
 namespace Microsoft.Win32.SafeHandles
 {
+    /// <summary>
+    /// Used to wrap handles gotten from OpenSCManager or OpenService
+    /// </summary>
     internal class SafeServiceHandle : SafeHandle
     {
         internal SafeServiceHandle(IntPtr handle) : base(IntPtr.Zero, true)

diff --git a/src/libraries/System.ServiceProcess.ServiceController/src/Resources/Strings.resx b/src/libraries/System.ServiceProcess.ServiceController/src/Resources/Strings.resx
@@ -62,10 +62,10 @@
     <value>Arguments within the 'args' array passed to Start cannot be null.</value>
   </data>
   <data name="BadMachineName" xml:space="preserve">
-    <value>MachineName value {0} is invalid.</value>
+    <value>MachineName '{0}' is invalid.</value>
   </data>
   <data name="CannotStart" xml:space="preserve">
-    <value>Cannot start service {0} on computer '{1}'.</value>
+    <value>Cannot start service '{0}' on computer '{1}'.</value>
   </data>
   <data name="InvalidEnumArgument" xml:space="preserve">
     <value>The value of argument '{0}' ({1}) is invalid for Enum type '{2}'.</value>
@@ -77,22 +77,22 @@
     <value>MachineName was not set.</value>
   </data>
   <data name="NoService" xml:space="preserve">
-    <value>Service {0} was not found on computer '{1}'.</value>
+    <value>Service '{0}' was not found on computer '{1}'.</value>
   </data>
   <data name="OpenSC" xml:space="preserve">
     <value>Cannot open Service Control Manager on computer '{0}'. This operation might require other privileges.</value>
   </data>
   <data name="OpenService" xml:space="preserve">
-    <value>Cannot open {0} service on computer '{1}'.</value>
+    <value>Cannot open '{0}' service on computer '{1}'.</value>
   </data>
   <data name="PauseService" xml:space="preserve">
-    <value>Cannot pause {0} service on computer '{1}'.</value>
+    <value>Cannot pause '{0}' service on computer '{1}'.</value>
   </data>
   <data name="ResumeService" xml:space="preserve">
-    <value>Cannot resume {0} service on computer '{1}'.</value>
+    <value>Cannot resume '{0}' service on computer '{1}'.</value>
   </data>
   <data name="StopService" xml:space="preserve">
-    <value>Cannot stop {0} service on computer '{1}'.</value>
+    <value>Cannot stop '{0}' service on computer '{1}'.</value>
   </data>
   <data name="Timeout" xml:space="preserve">
     <value>Time out has expired and the operation has not been completed.</value>
@@ -107,7 +107,7 @@
     <value>Cannot change service name when the service is running.</value>
   </data>
   <data name="ServiceName" xml:space="preserve">
-    <value>Service name {0} contains invalid characters, is empty, or is too long (max length = {1}).</value>
+    <value>Service name '{0}' contains invalid characters, is empty, or is too long (max length = {1}).</value>
   </data>
   <data name="NoServices" xml:space="preserve">
     <value>Service has not been supplied. At least one object derived from ServiceBase is required in order to run.</value>
@@ -179,18 +179,18 @@
     <value>Failed in handling the PowerEvent. The error that occurred was: {0}.</value>
   </data>
   <data name="InstallOK" xml:space="preserve">
-    <value>Service {0} has been successfully installed.</value>
+    <value>Service '{0}' has been successfully installed.</value>
   </data>
   <data name="TryToStop" xml:space="preserve">
-    <value>Attempt to stop service {0}.</value>
+    <value>Attempt to stop service '{0}'.</value>
   </data>
   <data name="ServiceRemoving" xml:space="preserve">
-    <value>Service {0} is being removed from the system...</value>
+    <value>Service '{0}' is being removed from the system...</value>
   </data>
   <data name="ServiceRemoved" xml:space="preserve">
-    <value>Service {0} was successfully removed from the system.</value>
+    <value>Service '{0}' was successfully removed from the system.</value>
   </data>
   <data name="ControlService" xml:space="preserve">
-    <value>Cannot control {0} service on computer '{1}'.</value>
+    <value>Cannot control '{0}' service on computer '{1}'.</value>
   </data>
 </root>
diff --git a/...ystem.ServiceProcess.ServiceController/src/System.ServiceProcess.ServiceController.csproj b/...ystem.ServiceProcess.ServiceController/src/System.ServiceProcess.ServiceController.csproj
@@ -12,6 +12,9 @@
     <Configurations>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</Configurations>
   </PropertyGroup>
   <ItemGroup Condition="$(TargetGroup.StartsWith('netcoreapp')) OR ('$(TargetGroup)' == 'netstandard' AND '$(TargetsWindows)' == 'true')">
+    <Compile Include="$(CommonPath)\CoreLib\System\Text\ValueStringBuilder.cs">
+      <Link>Common\CoreLib\System\Text\ValueStringBuilder.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)\Interop\Windows\Interop.Libraries.cs">
       <Link>Common\Interop\Windows\Interop.Libraries.cs</Link>
     </Compile>
@@ -33,6 +36,12 @@
     <Compile Include="$(CommonPath)\Interop\Windows\advapi32\Interop.EnumServicesStatusEx.cs">
       <Link>Common\Interop\Windows\Interop.EnumServicesStatusEx.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)\Interop\Windows\advapi32\Interop.GetServiceDisplayName.cs">
+      <Link>Common\Interop\Windows\Interop.GetServiceDisplayName.cs</Link>
+    </Compile>
+    <Compile Include="$(CommonPath)\Interop\Windows\advapi32\Interop.GetServiceKeyName.cs">
+      <Link>Common\Interop\Windows\Interop.GetServiceKeyName.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)\Interop\Windows\advapi32\Interop.OpenSCManager.cs">
       <Link>Common\Interop\Windows\Interop.OpenSCManager.cs</Link>
     </Compile>
@@ -93,13 +102,16 @@
   </ItemGroup>
   <ItemGroup Condition="$(TargetGroup.StartsWith('netcoreapp')) OR ('$(TargetGroup)' == 'netstandard' AND '$(TargetsWindows)' == 'true')">
     <Reference Include="Microsoft.Win32.Primitives" />
+    <Reference Include="System.Buffers" />
     <Reference Include="System.Collections" />
     <Reference Include="System.Console" />
     <Reference Include="System.ComponentModel.Primitives" />
     <Reference Include="System.Diagnostics.Debug" />
     <Reference Include="System.Diagnostics.Tools" />
+    <Reference Include="System.Memory" />
     <Reference Include="System.Resources.ResourceManager" />
     <Reference Include="System.Runtime" />
+    <Reference Include="System.Runtime.Extensions" />
     <Reference Include="System.Runtime.InteropServices" />
     <Reference Include="System.Threading" />
     <Reference Include="System.Threading.Thread" />