Skip to content

fix: UI timer disposal, PerformanceCounter blocking, and diagnostics #23

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.

Sign up for GitHub

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

Merged
beenycool merged 6 commits into from
Mar 20, 2026
Merged

fix: UI timer disposal, PerformanceCounter blocking, and diagnostics #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e22a98a
Optimize AsusControl by caching fan count to reduce hardware I/O
google-labs-jules[bot] Feb 5, 2026
0cb628a
Refactor fan count loops
beenycool Feb 6, 2026
9e0c992
fix: hardware race conditions and P/Invoke safety
beenycool Mar 17, 2026
290d831
fix: FanCurve thread safety with proper locking
beenycool Mar 17, 2026
6779a37
fix: UI timer disposal, PerformanceCounter blocking, and diagnostics
beenycool Mar 17, 2026
cc36cd1
Apply reviewer suggestions for PR #23
beenycool Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 76 additions & 62 deletions AsusFanControl.Core/AsusControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -14,13 +15,12 @@ public class AsusControl : IFanController, IDisposable
private const int ResetCommandDelayMs = 10;
private const int MonitorIntervalMs = 1000;

// Static lock to synchronize access to the shared hardware resource (driver/DLL state)
private static readonly object _hardwareLock = new object();
private static int _instanceCount = 0;

private readonly int _fanCount;
private bool _disposed = false;

// Cache for fan speeds to enable non-blocking reads
private volatile int[] _cachedFanSpeeds;
private CancellationTokenSource _cts;
private Task _monitorTask;
Expand All @@ -29,17 +29,19 @@ public AsusControl()
{
lock (_hardwareLock)
{
AsusWinIO64.InitializeWinIo();
if (_instanceCount == 0)
{
AsusWinIO64.InitializeWinIo();
}

_fanCount = AsusWinIO64.HealthyTable_FanCounts();
_instanceCount++;
}

_cachedFanSpeeds = new int[_fanCount];
_cts = new CancellationTokenSource();

// Perform initial read synchronously to populate cache immediately
UpdateFanSpeeds();

// Start background monitoring task
_monitorTask = Task.Run(() => MonitorLoop(_cts.Token));
}

Expand All @@ -56,51 +58,59 @@ public void Dispose()

protected virtual void Dispose(bool disposing)
{
if (!_disposed)
if (_disposed) return;

_cts?.Cancel();
if (disposing)
{
// Always stop the background task to prevent leak on non-disposing cleanup
_cts?.Cancel();
if (disposing)
try
{
_monitorTask?.Wait(2000);
}
catch (AggregateException ae)
{
ae.Handle(e => e is TaskCanceledException);
}

_cts?.Dispose();
}

lock (_hardwareLock)
{
if (_disposed) return;

_instanceCount--;
if (_instanceCount == 0)
{
try
{
_monitorTask?.Wait(2000); // Wait up to 2 seconds for clean exit
AsusWinIO64.ShutdownWinIo();
}
catch (AggregateException ae)
catch (Exception ex)
{
ae.Handle(e => e is TaskCanceledException);
Debug.WriteLine($"[AsusControl] Error shutting down WinIo: {ex.Message}");
}
_cts?.Dispose();
}

// Unmanaged resources
lock (_hardwareLock)
{
AsusWinIO64.ShutdownWinIo();
}
_disposed = true;
}
}

private void UpdateFanSpeeds()
{
if (_disposed) return;

// Create a local array to store new values
var newSpeeds = new int[_fanCount];

// Read each fan speed individually
// Locking per fan allows other operations (like SetFanSpeed) to interleave
for (byte i = 0; i < _fanCount; i++)
for (byte fanIndex = 0; fanIndex < _fanCount; fanIndex++)
{
lock (_hardwareLock)
{
AsusWinIO64.HealthyTable_SetFanIndex(i);
newSpeeds[i] = AsusWinIO64.HealthyTable_FanRPM();
if (_disposed) return;

AsusWinIO64.HealthyTable_SetFanIndex(fanIndex);
newSpeeds[fanIndex] = AsusWinIO64.HealthyTable_FanRPM();
}
}

// Atomically replace the reference to the cached array
_cachedFanSpeeds = newSpeeds;
}

Expand All @@ -122,16 +132,16 @@ private async Task MonitorLoop(CancellationToken token)
}
catch (Exception ex)
{
System.Diagnostics.Debug.WriteLine($"[AsusControl] Monitor loop error: {ex.Message}");
Debug.WriteLine($"[AsusControl] Monitor loop error: {ex.Message}");
}
}
}

private void SetFanSpeed(byte value, byte fanIndex = 0)
{
if (_disposed) return;
lock (_hardwareLock)
{
if (_disposed) return;
AsusWinIO64.HealthyTable_SetFanIndex(fanIndex);
AsusWinIO64.HealthyTable_SetFanTestMode(value > 0 ? FanModeManual : FanModeDefault);
AsusWinIO64.HealthyTable_SetFanPwmDuty(value);
Expand All @@ -148,52 +158,37 @@ private int ClampPercentage(int percent)
public void SetFanSpeed(int percent, byte fanIndex = 0)
{
if (_disposed) return;

percent = ClampPercentage(percent);
var value = (byte)(percent / 100.0f * 255);
SetFanSpeed(value, fanIndex);
}

private async Task SetFanSpeeds(byte value)
{
if (_disposed) return;
var fanCount = _fanCount;
for(byte fanIndex = 0; fanIndex < fanCount; fanIndex++)
{
// SetFanSpeed acquires the lock internally
SetFanSpeed(value, fanIndex);

// Keep delay to space out hardware commands if necessary
await Task.Delay(20);
}
}

public void SetFanSpeeds(int percent)
{
if (_disposed) return;

percent = ClampPercentage(percent);
var value = (byte)(percent / 100.0f * 255);
_ = SetFanSpeeds(value);
for (byte fanIndex = 0; fanIndex < _fanCount; fanIndex++)
{
SetFanSpeed(value, fanIndex);
}
}

public int GetFanSpeed(byte fanIndex = 0)
{
if (_disposed) return 0;

// Live read with lock
lock (_hardwareLock)
{
if (_disposed) return 0;
AsusWinIO64.HealthyTable_SetFanIndex(fanIndex);
var fanSpeed = AsusWinIO64.HealthyTable_FanRPM();
return fanSpeed;
return AsusWinIO64.HealthyTable_FanRPM();
}
}

public List<int> GetFanSpeeds()
{
if (_disposed) return new List<int>();

// Return a copy of the cached fan speeds
// This is non-blocking and instant (O(N) memory copy)
return new List<int>(_cachedFanSpeeds);
}

Expand All @@ -205,26 +200,45 @@ public int HealthyTable_FanCounts()

public ulong Thermal_Read_Cpu_Temperature()
{
if (_disposed) return 0;
lock (_hardwareLock)
{
if (_disposed) return 0;
return AsusWinIO64.Thermal_Read_Cpu_Temperature();
}
}

public Task ResetToDefaultAsync()
{
if (_disposed) return Task.CompletedTask;
// Synchronous reset for safety (e.g. ProcessExit)
var fanCount = _fanCount;
for(byte fanIndex = 0; fanIndex < fanCount; fanIndex++)
return Task.Run(() => ResetToDefault());
}
Comment on lines +210 to 214

@gemini-code-assist gemini-code-assist Bot Mar 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The method ResetToDefaultAsync is not truly asynchronous and has a potential thread-safety issue. It calls ResetToDefaultInternal() directly without acquiring the _hwLock, while other methods do. This could lead to race conditions. Additionally, it blocks the calling thread because ResetToDefaultInternal is synchronous.

To fix this, you should call the public ResetToDefault() method inside Task.Run to make it both thread-safe and truly asynchronous.

        public Task ResetToDefaultAsync()
        {
            if (_disposed) return Task.CompletedTask;
            return Task.Run(() => ResetToDefault());
        }


public void ResetToDefault()
{
lock (_hardwareLock)
{
if (_disposed) return;
ResetToDefaultInternal();
}
}

private void ResetToDefaultInternal()
{
for (byte fanIndex = 0; fanIndex < _fanCount; fanIndex++)
{
SetFanSpeed(0, fanIndex);
// Minimal blocking delay to ensure hardware processes the command if needed,
// but keep it fast for shutdown.
System.Threading.Thread.Sleep(ResetCommandDelayMs);
try
{
AsusWinIO64.HealthyTable_SetFanIndex(fanIndex);
AsusWinIO64.HealthyTable_SetFanTestMode(FanModeDefault);
AsusWinIO64.HealthyTable_SetFanPwmDuty(0);
}
catch (Exception ex)
{
Debug.WriteLine($"[AsusControl] Failed to reset fan {fanIndex}: {ex.Message}");
}

Thread.Sleep(ResetCommandDelayMs);
}
return Task.CompletedTask;
}
}
}
104 changes: 96 additions & 8 deletions AsusFanControl.Core/FanCurve.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,30 @@ public FanCurvePoint() { }

public class FanCurve
{
public List<FanCurvePoint> Points { get; set; } = new List<FanCurvePoint>();
private readonly object _lock = new object();
private List<FanCurvePoint> _points = new List<FanCurvePoint>();

public IReadOnlyList<FanCurvePoint> Points
{
get
{
lock (_lock)
{
return _points.ToList();
}
}
}

public int GetTargetSpeed(int currentTemp)
{
var points = Points;
if (points == null || points.Count == 0)
return 0;
List<FanCurvePoint> sortedPoints;
lock (_lock)
{
if (_points.Count == 0)
return 0;

List<FanCurvePoint> sortedPoints = points.ToList();
sortedPoints = _points.ToList();
}

bool isSorted = true;
for (int i = 0; i < sortedPoints.Count - 1; i++)
Expand Down Expand Up @@ -74,7 +89,77 @@ public int GetTargetSpeed(int currentTemp)

public override string ToString()
{
return string.Join(",", Points.Select(p => $"{p.Temperature}:{p.Speed}"));
List<FanCurvePoint> snapshot;
lock (_lock)
{
snapshot = _points.ToList();
}

return string.Join(",", snapshot.Select(p => $"{p.Temperature}:{p.Speed}"));
}

public void SetPoints(IEnumerable<FanCurvePoint> newPoints)
{
lock (_lock)
{
_points.Clear();
if (newPoints != null)
{
foreach (var point in newPoints)
{
_points.Add(new FanCurvePoint(point.Temperature, point.Speed));
}
}
}
}

public void AddPoint(FanCurvePoint point)
{
lock (_lock)
{
_points.Add(new FanCurvePoint(point.Temperature, point.Speed));
}
}

public void RemovePointAt(int index)
{
lock (_lock)
{
if (index >= 0 && index < _points.Count)
{
_points.RemoveAt(index);
}
}
}

public void UpdatePointAt(int index, FanCurvePoint point)
{
lock (_lock)
{
if (index >= 0 && index < _points.Count)
{
_points[index] = new FanCurvePoint(point.Temperature, point.Speed);
}
}
}

public int PointCount
{
get
{
lock (_lock)
{
return _points.Count;
}
}
}

public void ClearPoints()
{
lock (_lock)
{
_points.Clear();
}
}

public static FanCurve FromString(string data)
Expand All @@ -83,15 +168,18 @@ public static FanCurve FromString(string data)
if (string.IsNullOrWhiteSpace(data))
return curve;

var points = new List<FanCurvePoint>();
var parts = data.Split(',');
foreach (var part in parts)
{
var kv = part.Split(':');
if (kv.Length == 2 && int.TryParse(kv[0], out int t) && int.TryParse(kv[1], out int s))
if (kv.Length == 2 && int.TryParse(kv[0], out int temperature) && int.TryParse(kv[1], out int speed))
{
curve.Points.Add(new FanCurvePoint(t, s));
points.Add(new FanCurvePoint(temperature, speed));
}
}

curve.SetPoints(points);
return curve;
}
}
Expand Down
Loading
Loading