Skip to content

fix: sync-over-async, lazy enumeration, specific catches (CQ-004, CQ-010, CQ-011) #372

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
laurentiu021 merged 1 commit into from
May 15, 2026
Merged

fix: sync-over-async, lazy enumeration, specific catches (CQ-004, CQ-010, CQ-011) #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Binary file modified .gitignore
View file
Open in desktop
Binary file not shown.
13 changes: 13 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

## [0.48.11] - 2026-05-15

### Fixed
- **ProcessManagerViewModel (CQ-004)** — replaced sync-over-async
`GetAwaiter().GetResult()` with proper `await` inside `Task.Run` async
lambda, preventing thread pool thread blocking.
- **DeepCleanupService (CQ-010)** — replaced `Directory.GetFiles()` and
`GetDirectories()` (full array allocation) with lazy `EnumerateFiles()`
and `EnumerateDirectories()` to reduce memory pressure on large directories.
- **TracerouteService (CQ-011)** — bare `catch {}` replaced with specific
`PingException`, `SocketException`, `InvalidOperationException` catches;
subscriber error catch narrowed to `catch (Exception)`.

## [0.48.10] - 2026-05-15

### Fixed
Expand Down
12 changes: 6 additions & 6 deletions SysManager/SysManager/Services/DeepCleanupService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,10 @@ private static IEnumerable<string> EnumerateFiles(string root, CancellationToken
while (stack.Count > 0 && !ct.IsCancellationRequested)
{
var cur = stack.Pop();
string[] files = Array.Empty<string>();
string[] dirs = Array.Empty<string>();
try { files = Directory.GetFiles(cur); } catch (IOException) { } catch (UnauthorizedAccessException) { }
try { dirs = Directory.GetDirectories(cur); } catch (IOException) { } catch (UnauthorizedAccessException) { }
IEnumerable<string> files;
IEnumerable<string> dirs;
try { files = Directory.EnumerateFiles(cur); } catch (IOException) { continue; } catch (UnauthorizedAccessException) { continue; }
try { dirs = Directory.EnumerateDirectories(cur); } catch (IOException) { dirs = []; } catch (UnauthorizedAccessException) { dirs = []; }
foreach (var f in files) yield return f;
foreach (var d in dirs) stack.Push(d);
Comment on lines +381 to 384

@coderabbitai coderabbitai Bot May 15, 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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In .NET, can Directory.EnumerateFiles/Directory.EnumerateDirectories throw IOException or UnauthorizedAccessException during foreach iteration (MoveNext), not just when the enumerable is created?

💡 Result:

Yes, in.NET, Directory.EnumerateFiles and Directory.EnumerateDirectories can throw exceptions such as UnauthorizedAccessException and IOException during the foreach iteration (specifically when MoveNext is called), not just when the enumerable is initially created [1][2]. Because these methods use deferred execution—acting as wrappers around native file system enumeration APIs—they do not retrieve the entire list of files or directories upfront [3][4]. Instead, they fetch entries incrementally as you iterate [1][2]. Consequently, if the enumerator encounters a directory it cannot access, a file that has been locked, or other I/O-related issues while traversing the file system, the exception will be raised at the point of iteration [1][2]. To handle these scenarios robustly, you can use the EnumerationOptions class (available in modern.NET versions) to configure the behavior of the enumerator [5][6]. Specifically, setting the IgnoreInaccessible property to true allows the enumerator to skip files or directories that would otherwise trigger an UnauthorizedAccessException, enabling the iteration to continue [5][6]. For older versions of.NET or more complex error handling, you may need to implement custom logic to wrap the enumeration in a way that catches and handles these exceptions during the MoveNext process [7][1].

Citations:

🏁 Script executed:

find . -name "DeepCleanupService.cs" -type f

Repository: laurentiu021/SystemManager

Length of output: 125

🏁 Script executed:

sed -n '370,410p' ./SysManager/SysManager/Services/DeepCleanupService.cs | cat -n

Repository: laurentiu021/SystemManager

Length of output: 1961

Handle deferred Enumerate* exceptions inside iteration.

Directory.EnumerateFiles and Directory.EnumerateDirectories use deferred execution and can throw IOException or UnauthorizedAccessException during foreach iteration (via MoveNext), not just when the enumerable is created. The current exception handling only wraps the enumerable creation, leaving iteration unprotected.

Move the foreach loops inside the try blocks:

Proposed fix for EnumerateFiles 
-            IEnumerable<string> files;
-            IEnumerable<string> dirs;
-            try { files = Directory.EnumerateFiles(cur); } catch (IOException) { continue; } catch (UnauthorizedAccessException) { continue; }
-            try { dirs = Directory.EnumerateDirectories(cur); } catch (IOException) { dirs = []; } catch (UnauthorizedAccessException) { dirs = []; }
-            foreach (var f in files) yield return f;
-            foreach (var d in dirs) stack.Push(d);
+            IEnumerable<string> dirs = [];
+            try
+            {
+                foreach (var f in Directory.EnumerateFiles(cur))
+                    yield return f;
+            }
+            catch (IOException) { continue; }
+            catch (UnauthorizedAccessException) { continue; }
+
+            try { dirs = Directory.EnumerateDirectories(cur); }
+            catch (IOException) { dirs = []; }
+            catch (UnauthorizedAccessException) { dirs = []; }
+
+            foreach (var d in dirs) stack.Push(d);
Proposed fix for EnumerateDirectoriesDepthFirst 
-            IEnumerable<string> dirs;
-            try { dirs = Directory.EnumerateDirectories(cur); } catch (IOException) { continue; } catch (UnauthorizedAccessException) { continue; }
-            foreach (var d in dirs) stack.Push(d);
+            try
+            {
+                foreach (var d in Directory.EnumerateDirectories(cur))
+                    stack.Push(d);
+            }
+            catch (IOException) { continue; }
+            catch (UnauthorizedAccessException) { continue; }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SysManager/SysManager/Services/DeepCleanupService.cs` around lines 381 - 384,
The EnumerateFiles/EnumerateDirectories usage currently only catches exceptions
thrown when creating the IEnumerable but not those thrown during deferred
iteration; move each foreach into its own try/catch so iteration is protected:
for the files loop (in DeepCleanupService) wrap "foreach (var f in files) yield
return f;" inside a try that catches IOException and UnauthorizedAccessException
and continues on exception, and for the dirs loop wrap "foreach (var d in dirs)
stack.Push(d);" inside a try that on IOException/UnauthorizedAccessException
sets dirs to empty or simply continues; update the blocks around the variables
`files`, `dirs`, and the stack push to ensure iteration exceptions are handled
the same way as enumeration construction.

}
Expand All @@ -393,8 +393,8 @@ private static IEnumerable<string> EnumerateDirectoriesDepthFirst(string root, C
while (stack.Count > 0 && !ct.IsCancellationRequested)
{
var cur = stack.Pop();
string[] dirs = Array.Empty<string>();
try { dirs = Directory.GetDirectories(cur); } catch (IOException) { } catch (UnauthorizedAccessException) { }
IEnumerable<string> dirs;
try { dirs = Directory.EnumerateDirectories(cur); } catch (IOException) { continue; } catch (UnauthorizedAccessException) { continue; }
foreach (var d in dirs) stack.Push(d);
if (!string.Equals(cur, root, StringComparison.OrdinalIgnoreCase)) all.Add(cur);
}
Expand Down
6 changes: 4 additions & 2 deletions SysManager/SysManager/Services/TracerouteService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@
}
}
catch (OperationCanceledException) { throw; }
catch { /* swallow per-probe errors; we report them as a timeout */ }
catch (System.Net.NetworkInformation.PingException) { /* per-probe failure; reported as timeout */ }
catch (System.Net.Sockets.SocketException) { /* per-probe failure; reported as timeout */ }
catch (InvalidOperationException) { /* per-probe failure; reported as timeout */ }
}

var hop = new TracerouteHop
Expand Down Expand Up @@ -101,7 +103,7 @@
foreach (var h in handlers)
{
try { ((Action<TracerouteHop>)h).Invoke(hop); }
catch { /* swallow subscriber errors */ }
catch (Exception) { /* swallow subscriber errors to protect iteration */ }

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
}
}
}
4 changes: 2 additions & 2 deletions SysManager/SysManager/ViewModels/ProcessManagerViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ private async Task RefreshAsync()
{
// PERF-007: Perform snapshot, icon extraction, and description lookup
// on a background thread to avoid UI freezes on slow processes.
var enriched = await Task.Run(() =>
var enriched = await Task.Run(async () =>
{
var snapshot = _service.SnapshotAsync().GetAwaiter().GetResult();
var snapshot = await _service.SnapshotAsync();
foreach (var p in snapshot)
{
p.Icon = IconExtractorService.GetProcessIcon(p.FilePath, p.Name);
Expand Down
Loading