Skip to content

[Code Quality] NativeMethods.cs — Inconsistent SafeHandle vs bare IntPtr across SCM/Job P/Invokes #730

@Christophe-Rogiers

Description

@Christophe-Rogiers

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.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions