-
-
Notifications
You must be signed in to change notification settings - Fork 2
fix: dispose resource leaks in NetworkSharedState, TrayIconService, MemoryTestService #347
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🧩 Analysis chain
🏁 Script executed:
Repository: laurentiu021/SystemManager
Length of output: 127
🏁 Script executed:
cat -n ./SysManager/SysManager/ViewModels/NetworkSharedState.cs | head -100Repository: laurentiu021/SystemManager
Length of output: 4998
🏁 Script executed:
Repository: laurentiu021/SystemManager
Length of output: 131
🏁 Script executed:
Repository: laurentiu021/SystemManager
Length of output: 744
🏁 Script executed:
Repository: laurentiu021/SystemManager
Length of output: 572
🏁 Script executed:
rg "SolidColorPaint" ./SysManager/SysManager/ViewModels/NetworkSharedState.cs -B 2 -A 2Repository: laurentiu021/SystemManager
Length of output: 2272
🏁 Script executed:
Repository: laurentiu021/SystemManager
Length of output: 167
🌐 Web query:
SkiaSharp SKPaint Dispose IDisposable memory management💡 Result:
SKPaint in SkiaSharp implements IDisposable and must be explicitly disposed to release unmanaged native resources promptly, preventing memory leaks especially on resource-constrained devices like mobile.[1][2][3][4] The official documentation states: "Always dispose the object before you release your last reference to the SKPaint. Otherwise, the resources it is using will not be freed until the garbage collector calls the finalizer."[1][5] SKPaint inherits from SKNativeObject, which provides Dispose to release both managed and unmanaged resources, and a protected override Dispose(bool disposing) for the dispose pattern.[1][6][2] Failing to call Dispose delays native resource cleanup until GC finalization, which is non-deterministic and can lead to higher memory usage.[7][8][3] Best practices include: - Use 'using' statements for automatic disposal: using (var paint = new SKPaint) {... }[4] - Cache and reuse SKPaint objects for repeated use (e.g., same stroke color/style), as they are lightweight and do not maintain state like SKCanvas.[6][3][4] Dispose only when no longer needed. - Avoid recreating SKPaint in hot loops; dispose explicitly after caching or when changing properties significantly.[3][4] GC will eventually finalize undisposed objects, but explicit Dispose ensures deterministic cleanup.[9][8][3] Recent issues highlight keeping objects alive during native calls with GC.KeepAlive, but disposal remains essential post-use.[10]
Citations:
SKNativeObjectinstances are not kept alive during native API calls mono/SkiaSharp#2821🏁 Script executed:
Repository: laurentiu021/SystemManager
Length of output: 174
🌐 Web query:
LiveChartsCore.SkiaSharpView.Painting SolidColorPaint Dispose💡 Result:
In LiveCharts2, LiveChartsCore.SkiaSharpView.Painting.SolidColorPaint inherits Dispose/Dispose semantics from LiveChartsCore.SkiaSharpView.Painting.SkiaPaint, and you should not treat SolidColorPaint as something you manually dispose in normal UI usage. SolidColorPaint itself is a paint wrapper that sets its color during painting (it updates an internal Skia paint’s Color in OnPaintStarted/Transitionate) and does not create/dispose any Skia shader resources the way gradient paints do [1]. Disposal behavior: - The SolidColorPaint API docs show a Dispose member (inherited from its base) described as “freeing, releasing, or resetting unmanaged resources” [2]. - In the library source, gradient paints override DisposeTask to dispose their internal shader objects (example: LinearGradientPaint disposes _shader in both Transitionate and DisposeTask), which indicates why gradient paints may need disposal if you keep them around for a long time or create many of them [3]. SolidColorPaint does not show such shader disposal logic in its implementation [1]. Practical guidance: - If you pass SolidColorPaint instances as Fill/Stroke in your chart/series, LiveCharts is expected to manage their lifecycle; creating them once and reusing them is typically preferred to repeated allocations. - Only call Dispose if you explicitly created a paint and own its lifecycle outside LiveCharts (e.g., you are dynamically creating many paints and want deterministic cleanup). Otherwise, prefer letting LiveCharts manage them. If you tell me your UI framework (WPF/WinForms/MAUI/etc.) and whether you reuse the same SolidColorPaint instance or create one per render/update, I can give more targeted advice on whether disposing is likely to matter in your scenario.
Citations:
Dispose all owned paint resources, not only the typeface.
Line 492 fixes only part of LEAK-003.
NetworkSharedStateownsLegendTextPaint,LegendBackgroundPaint,TooltipTextPaint, andTooltipBackgroundPaint(lines 64-67), which should be disposed inDispose()as well. All four areSolidColorPaintinstances created and reused by the class; they should be disposed when the class is disposed.Proposed fix
public void Dispose() { Pinger.SampleReceived -= OnSample; TraceMonitor.RouteCompleted -= OnRouteCompleted; Pinger.Stop(); Pinger.Dispose(); TraceMonitor.Stop(); TraceMonitor.Dispose(); FlushTimer?.Stop(); // Dispose SKTypeface (unmanaged SkiaSharp memory) — LEAK-003 LegendTextPaint.SKTypeface?.Dispose(); + LegendTextPaint.Dispose(); + LegendBackgroundPaint.Dispose(); + TooltipTextPaint.Dispose(); + TooltipBackgroundPaint.Dispose(); }📝 Committable suggestion
🤖 Prompt for AI Agents