Severity: Info
File: src/Servy.Core/Native/NativeMethods.cs
Description:
The P/Invoke layer mixes two conventions for handle-returning Win32 functions. Some return wrapped types (e.g., OpenProcess returns a SafeHandle subclass), while others return bare IntPtr:
// Wrapped / SafeHandle-style
[DllImport(...)] public static extern SafeProcessHandle OpenProcess(...);
// Bare IntPtr — manual CloseServiceHandle/CloseHandle required by caller
[DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
public static extern IntPtr OpenSCManager(string machineName, string databaseName, uint dwAccess);
[DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
public static extern IntPtr CreateService(...);
[DllImport("advapi32.dll", SetLastError = true)]
public static extern IntPtr OpenService(...);
[DllImport("kernel32.dll", SetLastError = true)]
public static extern IntPtr CreateJobObject(...);
Callers of SCM and JobObject APIs now have to remember (and document) that they own the handle and must call the matching CloseServiceHandle / CloseHandle in finally. The mixed surface makes it easy to introduce a leak when someone copies an OpenProcess-style call site and forgets the manual close.
Suggested fix:
Either (a) wrap each remaining IntPtr handle in a dedicated SafeHandle subclass (SafeServiceHandle, SafeJobObjectHandle, SafeScmHandle) so the GC and using blocks enforce disposal, or (b) add a large <remarks> block to each bare-IntPtr declaration spelling out the close-function and who is responsible. Option (a) is strongly preferred — the SafeHandle subclasses are short (8–15 lines each) and remove a whole class of leaks.
Severity: Info
File:
src/Servy.Core/Native/NativeMethods.csDescription:
The P/Invoke layer mixes two conventions for handle-returning Win32 functions. Some return wrapped types (e.g.,
OpenProcessreturns a SafeHandle subclass), while others return bareIntPtr:Callers of SCM and JobObject APIs now have to remember (and document) that they own the handle and must call the matching
CloseServiceHandle/CloseHandleinfinally. The mixed surface makes it easy to introduce a leak when someone copies anOpenProcess-style call site and forgets the manual close.Suggested fix:
Either (a) wrap each remaining IntPtr handle in a dedicated
SafeHandlesubclass (SafeServiceHandle,SafeJobObjectHandle,SafeScmHandle) so the GC andusingblocks enforce disposal, or (b) add a large<remarks>block to each bare-IntPtr declaration spelling out the close-function and who is responsible. Option (a) is strongly preferred — the SafeHandle subclasses are short (8–15 lines each) and remove a whole class of leaks.