-
-
Notifications
You must be signed in to change notification settings - Fork 2
fix: thread safety, event marshaling, entry-point robustness, config alignment #396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,15 @@ public partial class MainWindow : Window | |
| public MainWindow() | ||
| { | ||
| InitializeComponent(); | ||
|
|
||
| // Ensure ViewModel disposal even if OnClosed is not called (e.g. app shutdown) | ||
| if (Application.Current != null) | ||
| Application.Current.Exit += OnApplicationExit; | ||
| } | ||
|
|
||
| private void OnApplicationExit(object sender, ExitEventArgs e) | ||
| { | ||
| (DataContext as MainWindowViewModel)?.Dispose(); | ||
| } | ||
|
Comment on lines
+21
to
29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify whether MainWindowViewModel.Dispose() is idempotent and whether duplicate disposal is currently possible.
set -euo pipefail
echo "== MainWindow exit/dispose call sites =="
fd -i '^MainWindow\.xaml\.cs$' --exec rg -n -C3 --type=cs 'Exit \+=|OnApplicationExit|OnClosed|Dispose\s*\(' {}
echo
echo "== MainWindowViewModel dispose implementation =="
for f in $(fd -i '^MainWindowViewModel\.cs$'); do
echo "-- $f --"
rg -n -C5 --type=cs '\bclass\s+MainWindowViewModel\b|\bvoid\s+Dispose\s*\(|\bbool\s+_disposed\b|Interlocked|GC\.SuppressFinalize' "$f"
done
echo
echo "Expected verification result:"
echo "1) Dispose implementation has an idempotency guard (_disposed/Interlocked/etc.), OR"
echo "2) MainWindow is updated to ensure dispose is invoked exactly once."Repository: laurentiu021/SystemManager Length of output: 2067 Add an idempotency guard to On normal shutdown, both Either:
🤖 Prompt for AI Agents |
||
|
|
||
| /// <summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ namespace SysManager.Services; | |
| public sealed class PerformanceService | ||
| { | ||
| private readonly PowerShellRunner _ps; | ||
| private readonly SemaphoreSlim _psGate = new(1, 1); | ||
|
|
||
| // ── Well-known power plan GUIDs ── | ||
| internal const string BalancedGuid = "381b4222-f694-41f0-9685-ff5bb260df2e"; | ||
|
|
@@ -163,11 +164,12 @@ public async Task<PerformanceProfile> ReadProfileAsync(CancellationToken ct = de | |
| /// <summary>Parse active plan from powercfg /getactivescheme output.</summary> | ||
| public async Task<(string Name, string Guid)> GetActivePlanAsync(CancellationToken ct = default) | ||
| { | ||
| await _psGate.WaitAsync(ct).ConfigureAwait(false); | ||
| var lines = new List<string>(); | ||
| void OnLine(PowerShellLine l) => lines.Add(l.Text); | ||
| _ps.LineReceived += OnLine; | ||
| try { await _ps.RunProcessAsync("powercfg.exe", "/getactivescheme", ct, PowerShellRunner.OemEncoding); } | ||
| finally { _ps.LineReceived -= OnLine; } | ||
| finally { _ps.LineReceived -= OnLine; _psGate.Release(); } | ||
|
Comment on lines
+167
to
+172
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap handler subscription in the same Right now, Suggested pattern (apply to all four methods) await _psGate.WaitAsync(ct).ConfigureAwait(false);
var lines = new List<string>();
void OnLine(PowerShellLine l) => lines.Add(l.Text);
-_ps.LineReceived += OnLine;
-try { await _ps.RunProcessAsync("powercfg.exe", "/getactivescheme", ct, PowerShellRunner.OemEncoding); }
-finally { _ps.LineReceived -= OnLine; _psGate.Release(); }
+var subscribed = false;
+try
+{
+ _ps.LineReceived += OnLine;
+ subscribed = true;
+ await _ps.RunProcessAsync("powercfg.exe", "/getactivescheme", ct, PowerShellRunner.OemEncoding).ConfigureAwait(false);
+}
+finally
+{
+ if (subscribed) _ps.LineReceived -= OnLine;
+ _psGate.Release();
+}Also applies to: 217-220, 238-243, 468-477 🤖 Prompt for AI Agents |
||
|
|
||
| return ParseActivePlan(lines); | ||
| } | ||
|
|
@@ -212,9 +214,10 @@ public async Task<string> EnsureUltimatePerformancePlanAsync(CancellationToken c | |
|
|
||
| var lines = new List<string>(); | ||
| void OnLine(PowerShellLine l) => lines.Add(l.Text); | ||
| await _psGate.WaitAsync(ct).ConfigureAwait(false); | ||
| _ps.LineReceived += OnLine; | ||
| try { await _ps.RunProcessAsync("powercfg.exe", $"-duplicatescheme {UltimatePerfScheme}", ct, PowerShellRunner.OemEncoding); } | ||
| finally { _ps.LineReceived -= OnLine; } | ||
| finally { _ps.LineReceived -= OnLine; _psGate.Release(); } | ||
|
|
||
| // Parse GUID from output: "Power Scheme GUID: <guid> (Ultimate Performance)" | ||
| foreach (var line in lines) | ||
|
|
@@ -232,11 +235,12 @@ public async Task<string> EnsureUltimatePerformancePlanAsync(CancellationToken c | |
| /// <summary>Find a plan GUID by name substring.</summary> | ||
| public async Task<string?> FindPlanGuidByNameAsync(string nameSubstring, CancellationToken ct = default) | ||
| { | ||
| await _psGate.WaitAsync(ct).ConfigureAwait(false); | ||
| var lines = new List<string>(); | ||
| void OnLine(PowerShellLine l) => lines.Add(l.Text); | ||
| _ps.LineReceived += OnLine; | ||
| try { await _ps.RunProcessAsync("powercfg.exe", "/list", ct, PowerShellRunner.OemEncoding); } | ||
| finally { _ps.LineReceived -= OnLine; } | ||
| finally { _ps.LineReceived -= OnLine; _psGate.Release(); } | ||
|
|
||
| return ParsePlanGuidByName(lines, nameSubstring); | ||
| } | ||
|
|
@@ -461,6 +465,7 @@ public static bool SetGpuMaxPerformance(string subKey, bool maxPerformance) | |
| /// <summary>Read the current processor minimum state percentage (AC).</summary> | ||
| internal async Task<int> ReadProcessorMinPercentAsync(CancellationToken ct = default) | ||
| { | ||
| await _psGate.WaitAsync(ct).ConfigureAwait(false); | ||
| var lines = new List<string>(); | ||
| void OnLine(PowerShellLine l) => lines.Add(l.Text); | ||
| _ps.LineReceived += OnLine; | ||
|
|
@@ -469,7 +474,7 @@ internal async Task<int> ReadProcessorMinPercentAsync(CancellationToken ct = def | |
| await _ps.RunProcessAsync("powercfg.exe", | ||
| "/query SCHEME_CURRENT SUB_PROCESSOR PROCTHROTTLEMIN", ct, PowerShellRunner.OemEncoding).ConfigureAwait(false); | ||
| } | ||
| finally { _ps.LineReceived -= OnLine; } | ||
| finally { _ps.LineReceived -= OnLine; _psGate.Release(); } | ||
|
|
||
| return ParseProcessorMinPercent(lines); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return immediately after a successful pipe signal.
Lines 96-104 still fall through into the process/window scan after
client.Connect(...)succeeds, so a normal second launch can trigger both activation paths instead of using the Win32 path only as fallback. Adding an earlyreturn;keeps the old logic as a true backup path.Proposed fix
try { using var client = new NamedPipeClientStream(".", PipeName, PipeDirection.Out); client.Connect(timeout: 2000); // Connection alone signals the running instance to activate + return; }📝 Committable suggestion
🤖 Prompt for AI Agents