Fix IDisposable usage #411
Conversation
This PR fixes a memory leak in FontReader with Woff2 fonts. Because of this FontReader class now implements IDisposable interface. This PR also wraps disposable types with using statements to remove warnings.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #411 +/- ##
=====================================
Coverage 86% 86%
=====================================
Files 238 238
Lines 15257 15263 +6
Branches 2108 2109 +1
=====================================
+ Hits 13240 13247 +7
+ Misses 1546 1545 -1
Partials 471 471
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -26,7 +26,7 @@ public Buffer(int length) | |||
| this.buffer = ArrayPool<byte>.Shared.Rent(bufferSizeInBytes); | |||
| this.length = length; | |||
|
|
|||
| ByteMemoryManager<T> manager = new(this.buffer); | |||
| using ByteMemoryManager<T> manager = new(this.buffer); | |||
There was a problem hiding this comment.
Dispose() doesn't actually do anything for this type but given that is an implementation detail disposing is probably appropriate.
However, the manager should be held as a private field of the parent Buffer<T> and disposed at the end of the parent lifecycle.
There was a problem hiding this comment.
Sorry, didn't get the idea about the purpose of private field. You can contribute to my repo if you wish.
Thank you for your hard work! I'll continue with disposable in |
This PR fixes a memory leak in
FontReaderclass withWoff2fonts. Because of thisFontReaderclass now implementsIDisposableinterface.This PR also wraps disposable types with using statements to remove warnings.