From 39fffafa032c1acf0d5353b1fb5dd3e32c349ba4 Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Thu, 6 Aug 2026 19:19:46 +0300 Subject: [PATCH] fix: bound the log file and stop the Volume Control tab flooding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1645. The sink passed no fileSizeLimitBytes and no rollOnFileSizeLimit, so it took Serilog's defaults — 1 GB per file with rolling OFF — and the only bound on the folder was the 14-FILE count. Measured against the real sink rather than reasoning from the docs: 4000 lines of ~4 KB produced ONE 15 MB file with no roll. With the limit and rollOnFileSizeLimit it produces two files, the largest exactly 10 MB. That matters because the documented support path is "attach the log" (SUPPORT.md and the bug-report template), so an unattachable file breaks the evidence trail exactly when it is needed — and Debug is a real volume tier here, 290 Log.Debug call sites, some in loops. 10 MB x 14 files is a predictable ~140 MB worst case that also respects the low-end laptop this app targets. The issue placed the fix on a plain WriteTo.File call; it is actually inside the custom UserPathScrubbingSink's inner logger, which is where it went. MaxLogFileBytes and RetainedFileCount are now named constants, so Init and the sink cannot drift apart and the tests assert the same values the app uses. Also stopped the flood at source. AudioMixerService.GetPeak logged every COMException at Debug, and it is driven by the Volume Control tab's 50 ms peak-meter timer — one bad audio session wrote ~20 identical lines a second for as long as the tab stayed open, which both fills the file and pushes out everything worth reading. Now logged once per session-handle generation, mirroring the existing _routingProbed one-shot pattern, and reset in ReleaseGroups so a genuinely new failure after a device change is still reported. Tests: three added to the existing sink suite, which already drives the real private sink type by reflection into a temp file. Two pin the intent (small enough to attach to an issue at GitHub's 25 MB ceiling; whole folder bounded) and one drives the sink hard and asserts no produced file exceeds the limit — asserting the constants alone would have passed even with the arguments still missing, which was the actual defect. Not covered by a test: the GetPeak guard needs real COM audio sessions failing, which a unit test cannot produce. Stated rather than implied. All four projects rebuild with 0 warnings. --- CHANGELOG.md | 6 ++ .../LogServiceSinkScrubbingTests.cs | 57 +++++++++++++++++++ .../SysManager/Services/AudioMixerService.cs | 22 ++++++- SysManager/SysManager/Services/LogService.cs | 21 ++++++- SysManager/SysManager/SysManager.csproj | 6 +- 5 files changed, 107 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a2f4d7..7a981ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.57.8] - 2026-08-06 + +### Fixed +- **The log file can no longer grow big enough to be unusable.** SysManager keeps a diary of what it does, and that file is what you attach when reporting a problem. It had no size limit — one day's file was allowed to reach a gigabyte, which is far past what you could upload anywhere. Each file is now capped at 10 MB and a fresh one is started when it fills, keeping at most two weeks of them, so the diary stays small enough to send and takes a predictable amount of disk. +- **The Volume Control tab no longer floods that log.** If an app's audio could not be read, the tab wrote the same complaint twenty times a second for as long as it stayed open — filling the file with one repeated line and pushing out everything actually worth reading. It is now noted once, and again only if something genuinely changes. + ## [1.57.7] - 2026-08-06 ### Fixed diff --git a/SysManager/SysManager.Tests/LogServiceSinkScrubbingTests.cs b/SysManager/SysManager.Tests/LogServiceSinkScrubbingTests.cs index f620732..ab6ee3d 100644 --- a/SysManager/SysManager.Tests/LogServiceSinkScrubbingTests.cs +++ b/SysManager/SysManager.Tests/LogServiceSinkScrubbingTests.cs @@ -196,4 +196,61 @@ public void Sink_IsSafeUnderConcurrentWrites() // Every line is individually intact — no two events rendered into each other. Assert.All(lines, line => Assert.Matches(@"\[INF\] Line \d+ at ", line)); } + + // ── The log file is bounded ────────────────────────────────────────────── + // The sink passed no fileSizeLimitBytes and no rollOnFileSizeLimit, so it took Serilog's + // defaults: 1 GB per file with rolling OFF. The only bound on the folder was the 14-FILE count, + // so one daily file could grow to a gigabyte. Debug is a real volume tier here (290 Log.Debug + // call sites) and the documented support path is "attach the log" — an unattachable file breaks + // the evidence trail exactly when it is needed. + + [Fact] + public void TheSizeLimit_IsSmallEnoughToAttachToABugReport() + { + // 25 MB is GitHub's per-file attachment ceiling. A log the user cannot upload is the failure + // this bound exists to prevent, so the intent is pinned rather than just the number. + Assert.True(LogService.MaxLogFileBytes <= 25L * 1024 * 1024, + $"A {LogService.MaxLogFileBytes / 1024 / 1024} MB log file is too large to attach to an issue."); + Assert.True(LogService.MaxLogFileBytes >= 1024 * 1024, + "Too small to hold useful context for a crash report."); + } + + [Fact] + public void TheWholeLogFolder_IsBounded() + { + // The point of the pair: per-file ceiling × retained count is a predictable worst case, + // which is what the 14-file-count-alone version never gave. + var worstCase = LogService.MaxLogFileBytes * LogService.RetainedFileCount; + Assert.True(worstCase <= 250L * 1024 * 1024, + $"Worst-case log folder is {worstCase / 1024 / 1024} MB — too much for a low-end laptop."); + } + + [Fact] + public void TheSink_KeepsEveryFileUnderTheLimit() + { + // Asserting the constants alone would pass even if the sink never received them — the defect + // was a missing ARGUMENT, not a wrong number. So drive the real sink hard and check the files + // it actually produced. + // + // RollingInterval.Infinite so the only thing that could create a second file is the size + // limit: with a daily interval a date change could produce one and this would pass for the + // wrong reason. + var sinkType = typeof(LogService).GetNestedType("UserPathScrubbingSink", BindingFlags.NonPublic)!; + var path = Path.Combine(_dir, "roll.log"); + var sink = (ILogEventSink)Activator.CreateInstance( + sinkType, [path, RollingInterval.Infinite, LogService.RetainedFileCount])!; + + using (var logger = new LoggerConfiguration().MinimumLevel.Debug().WriteTo.Sink(sink).CreateLogger()) + { + var filler = new string('x', 1024); // ~1 KB per line + for (int i = 0; i < 2000; i++) logger.Information("{Index} {Filler}", i, filler); + } + (sink as IDisposable)?.Dispose(); + + var written = Directory.GetFiles(_dir, "roll*.log"); + Assert.NotEmpty(written); + Assert.All(written, f => + Assert.True(new FileInfo(f).Length <= LogService.MaxLogFileBytes + 64 * 1024, + $"{Path.GetFileName(f)} is {new FileInfo(f).Length} bytes, over the {LogService.MaxLogFileBytes}-byte limit.")); + } } diff --git a/SysManager/SysManager/Services/AudioMixerService.cs b/SysManager/SysManager/Services/AudioMixerService.cs index 2a6073e..628f37c 100644 --- a/SysManager/SysManager/Services/AudioMixerService.cs +++ b/SysManager/SysManager/Services/AudioMixerService.cs @@ -287,12 +287,29 @@ public float GetPeak(string sessionId) { if (meter.GetPeakValue(out float value) == 0 && value > peak) peak = value; } - catch (COMException ex) { Log.Debug("GetPeak failed: {Error}", ex.Message); } + catch (COMException ex) + { + // Log once per session-handle generation, not per call. GetPeak is driven by the + // Volume Control tab's 50 ms peak-meter timer, so one bad audio session wrote + // ~20 identical Debug lines a second for as long as the tab stayed open — the + // log's single biggest flood path. The message is identical every tick, so + // repeating it adds nothing; ReleaseGroups resets the flag, meaning a genuinely + // new failure after a device change is still reported. Mirrors the existing + // one-shot probe pattern (_routingProbed / _routingSupported). + if (!_peakFailureLogged) + { + _peakFailureLogged = true; + Log.Debug("GetPeak failed: {Error}", ex.Message); + } + } } return peak; } } + // Reset by ReleaseGroups, so it is per session-handle generation rather than per process. + private bool _peakFailureLogged; + // ── Output-device enumeration (documented API) ───────────────────────── /// @@ -492,6 +509,9 @@ private void ReleaseGroups() foreach (var control in controls) Release(control); _groups.Clear(); + // New handles mean a new chance to succeed — and a failure against them is new information, + // so allow it to be logged once more rather than staying silent for the whole session. + _peakFailureLogged = false; } private static void Release(object? comObject) diff --git a/SysManager/SysManager/Services/LogService.cs b/SysManager/SysManager/Services/LogService.cs index 063d51b..439cc8b 100644 --- a/SysManager/SysManager/Services/LogService.cs +++ b/SysManager/SysManager/Services/LogService.cs @@ -17,6 +17,16 @@ public static partial class LogService { public static Logger? Logger { get; private set; } + /// + /// Per-file ceiling for the rolling log. With this bounds the + /// whole folder at roughly 140 MB, and keeps any single file small enough to attach to a bug + /// report — which is the documented way a user sends us evidence. + /// + internal const long MaxLogFileBytes = 10L * 1024 * 1024; + + /// How many rolled files to keep. Combines with . + internal const int RetainedFileCount = 14; + public static string LogDir { get; } = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "SysManager", "logs"); @@ -59,7 +69,7 @@ public static void Init() .WriteTo.Sink(new UserPathScrubbingSink( Path.Combine(LogDir, "sysmanager-.log"), rollingInterval: RollingInterval.Day, - retainedFileCountLimit: 14)) + retainedFileCountLimit: RetainedFileCount)) .CreateLogger(); Log.Logger = Logger; Logger.Information("SysManager started"); @@ -98,6 +108,15 @@ public UserPathScrubbingSink( path, rollingInterval: rollingInterval, retainedFileCountLimit: retainedFileCountLimit, + // Bound each file AND roll when it fills. Without both, the sink took Serilog's + // defaults — 1 GB per file with rollOnFileSizeLimit false — so a single daily + // file could grow to a gigabyte with nothing rolling below it, and the only + // bound on the folder was the 14-FILE count. That matters because the whole + // support path is "attach the log" (SUPPORT.md, the bug-report template): a file + // too large to upload breaks the evidence trail exactly when it is needed, and + // Debug is a real volume tier here (290 Log.Debug call sites, some in loops). + fileSizeLimitBytes: MaxLogFileBytes, + rollOnFileSizeLimit: true, outputTemplate: "{Message:lj}{NewLine}") .CreateLogger(); } diff --git a/SysManager/SysManager/SysManager.csproj b/SysManager/SysManager/SysManager.csproj index bcb4544..808f8f2 100644 --- a/SysManager/SysManager/SysManager.csproj +++ b/SysManager/SysManager/SysManager.csproj @@ -10,9 +10,9 @@ SysManager true NU1603;NU1701 - 1.57.7 - 1.57.7.0 - 1.57.7.0 + 1.57.8 + 1.57.8.0 + 1.57.8.0 SysManager SysManager — Windows system monitoring toolkit by laurentiu021. Network, updates, health, logs, safe deep cleanup. https://github.com/laurentiu021/SystemManager