Address review findings for thread control and process discovery

- Drop the false WOW64 claim from GetContext32/SetContext32 docs and guard them for 32-bit targets only.\n- Make FrozenThread dispose the thread handles it owns; make Freeze(predicate) dispose filtered-out threads.\n- Pass the already-validated handle through GetThreadById instead of opening a second one.\n- Add no-progress guard to MemoryBase.EnumerateRegions.\n- Dispose unmatched Process candidates in ApplicationFinder.OpenProcess.\n- Clean up RemoteThreadExecutor allocation formatting.
This commit is contained in:
kbe
2026-07-22 17:18:48 +02:00
parent 3e294dc846
commit 1169fdb994
6 changed files with 78 additions and 19 deletions
+2 -1
View File
@@ -509,5 +509,6 @@ public sealed class RemoteThreadExecutor
} }
} }
return IntPtr.Zero; } return IntPtr.Zero;
}
} }
+5 -1
View File
@@ -308,7 +308,11 @@ public abstract class MemoryBase : IDisposable
yield break; yield break;
yield return new MemoryRegion(info); yield return new MemoryRegion(info);
address = info.BaseAddress + (nint)info.RegionSize; IntPtr next = info.BaseAddress + (nint)info.RegionSize;
if (next.ToInt64() <= address.ToInt64())
yield break;
address = next;
} }
} }
+10 -3
View File
@@ -41,12 +41,19 @@ public static class ApplicationFinder
if (candidates.Length > 1) if (candidates.Length > 1)
{ {
string list = string.Join(", ", candidates.Select(p => $"{p.ProcessName}:{p.Id}"));
foreach (Process candidate in candidates)
candidate.Dispose();
throw new InvalidOperationException( throw new InvalidOperationException(
$"Process name '{processName}' is ambiguous ({candidates.Length} matches): " + $"Process name '{processName}' is ambiguous ({candidates.Length} matches): {list}");
string.Join(", ", candidates.Select(p => $"{p.ProcessName}:{p.Id}")));
} }
return candidates[0]; Process result = candidates[0];
for (int i = 1; i < candidates.Length; i++)
candidates[i].Dispose();
return result;
} }
/// <summary> /// <summary>
+8 -5
View File
@@ -7,7 +7,7 @@ namespace WhiteMagic.Thread;
/// <summary> /// <summary>
/// A disposable scope that tracks a set of threads frozen by <see cref="ThreadFactory.Freeze"/>. /// A disposable scope that tracks a set of threads frozen by <see cref="ThreadFactory.Freeze"/>.
/// Disposing the scope resumes exactly those threads, in reverse order, even if the guarded /// Disposing the scope resumes exactly those threads, in reverse order, even if the guarded
/// body throws. /// body throws, and then disposes the underlying thread handles.
/// </summary> /// </summary>
public sealed class FrozenThread : IDisposable public sealed class FrozenThread : IDisposable
{ {
@@ -23,8 +23,7 @@ public sealed class FrozenThread : IDisposable
public IEnumerable<RemoteThread> Threads => _threads; public IEnumerable<RemoteThread> Threads => _threads;
/// <summary> /// <summary>
/// Resumes the frozen threads in reverse order. The original call is responsible for /// Resumes the frozen threads in reverse order, then disposes every thread handle.
/// disposing the <see cref="RemoteThread"/> instances afterwards.
/// </summary> /// </summary>
public void Dispose() public void Dispose()
{ {
@@ -41,9 +40,13 @@ public sealed class FrozenThread : IDisposable
} }
catch catch
{ {
// Resume-on-dispose is best-effort; callers keep the thread handles so // Resume-on-dispose is best-effort; the handle is still disposed below.
// they can diagnose or recover separately.
} }
} }
foreach (RemoteThread thread in _threads)
{
thread.Dispose();
}
} }
} }
+13 -3
View File
@@ -139,11 +139,17 @@ public sealed class RemoteThread : IDisposable
} }
/// <summary> /// <summary>
/// Reads the 32-bit native context of the thread. Valid for 32-bit targets or /// Reads the 32-bit native context of the thread. Valid only for 32-bit targets.
/// WOW64 threads selected by a 64-bit caller.
/// </summary> /// </summary>
public void GetContext32(out Context32 context) public void GetContext32(out Context32 context)
{ {
if (_memory.Is64Bit)
{
context = default;
throw new InvalidOperationException(
"Use GetContext64 for 64-bit targets; GetContext32 is valid for 32-bit targets only.");
}
context = new Context32 { ContextFlags = ContextFlags.X86Full }; context = new Context32 { ContextFlags = ContextFlags.X86Full };
if (!NativeMethods.GetThreadContext(_handle, ref context)) if (!NativeMethods.GetThreadContext(_handle, ref context))
{ {
@@ -153,10 +159,14 @@ public sealed class RemoteThread : IDisposable
} }
/// <summary> /// <summary>
/// Writes the 32-bit native context of the thread. /// Writes the 32-bit native context of the thread. Valid only for 32-bit targets.
/// </summary> /// </summary>
public void SetContext32(ref Context32 context) public void SetContext32(ref Context32 context)
{ {
if (_memory.Is64Bit)
throw new InvalidOperationException(
"Use SetContext64 for 64-bit targets; SetContext32 is valid for 32-bit targets only.");
if (!NativeMethods.SetThreadContext(_handle, ref context)) if (!NativeMethods.SetThreadContext(_handle, ref context))
{ {
int error = Marshal.GetLastPInvokeError(); int error = Marshal.GetLastPInvokeError();
+40 -6
View File
@@ -86,7 +86,13 @@ public sealed class ThreadFactory
if (threadId <= 0) if (threadId <= 0)
throw new ArgumentException("Thread ID must be positive.", nameof(threadId)); throw new ArgumentException("Thread ID must be positive.", nameof(threadId));
SafeMemoryHandle handle = NativeMethods.OpenThread(ThreadAccess.QueryInformation, false, threadId); const ThreadAccess requiredAccess =
ThreadAccess.SuspendResume |
ThreadAccess.GetContext |
ThreadAccess.SetContext |
ThreadAccess.QueryInformation;
SafeMemoryHandle handle = NativeMethods.OpenThread(requiredAccess, false, threadId);
if (handle.IsInvalid) if (handle.IsInvalid)
{ {
int error = Marshal.GetLastPInvokeError(); int error = Marshal.GetLastPInvokeError();
@@ -115,12 +121,13 @@ public sealed class ThreadFactory
$"Thread {threadId} does not belong to process {_memory.ProcessId}."); $"Thread {threadId} does not belong to process {_memory.ProcessId}.");
} }
// Open a handle with the rights the public RemoteThread surface needs. // Ownership of the validated handle transfers to the RemoteThread.
return new RemoteThread(_memory, threadId); return new RemoteThread(_memory, threadId, handle);
} }
finally catch
{ {
handle.Dispose(); handle.Dispose();
throw;
} }
} }
@@ -202,12 +209,39 @@ public sealed class ThreadFactory
} }
/// <summary> /// <summary>
/// Suspends all threads selected by <paramref name="predicate"/>. /// Suspends all target threads selected by <paramref name="predicate"/>.
/// </summary> /// </summary>
public FrozenThread Freeze(Func<RemoteThread, bool> predicate) public FrozenThread Freeze(Func<RemoteThread, bool> predicate)
{ {
ArgumentNullException.ThrowIfNull(predicate); ArgumentNullException.ThrowIfNull(predicate);
return Freeze(Enumerate().Where(predicate));
var selected = new List<RemoteThread>();
try
{
foreach (RemoteThread thread in Enumerate())
{
try
{
if (predicate(thread))
selected.Add(thread);
else
thread.Dispose();
}
catch
{
thread.Dispose();
throw;
}
}
return Freeze(selected);
}
catch
{
foreach (RemoteThread thread in selected)
thread.Dispose();
throw;
}
} }
private long GetCreationTime(int threadId) private long GetCreationTime(int threadId)