Pārlūkot izejas kodu

Fixed memory leak related to rendering Text elements with page numbers. Added finalizer warnings to various classes to ensure proper disposal of resources.

Marcin Ziąbek 9 mēneši atpakaļ
vecāks
revīzija
a8e311498b
31 mainītis faili ar 120 papildinājumiem un 28 dzēšanām
  1. 1 0
      Source/QuestPDF/Drawing/CompanionCanvas.cs
  2. 1 0
      Source/QuestPDF/Drawing/ImageCanvas.cs
  3. 1 0
      Source/QuestPDF/Drawing/Proxy/SnapshotRecorder.cs
  4. 1 0
      Source/QuestPDF/Drawing/SkiaDocumentCanvasBase.cs
  5. 1 3
      Source/QuestPDF/Drawing/SvgCanvas.cs
  6. 1 0
      Source/QuestPDF/Elements/DynamicImage.cs
  7. 1 0
      Source/QuestPDF/Elements/DynamicSvgImage.cs
  8. 9 3
      Source/QuestPDF/Elements/Image.cs
  9. 10 4
      Source/QuestPDF/Elements/SvgImage.cs
  10. 7 0
      Source/QuestPDF/Elements/Text/TextBlock.cs
  11. 1 0
      Source/QuestPDF/Infrastructure/Image.cs
  12. 1 0
      Source/QuestPDF/Infrastructure/SvgImage.cs
  13. 25 4
      Source/QuestPDF/Infrastructure/TextStyle.cs
  14. 6 9
      Source/QuestPDF/Infrastructure/TextStyleManager.cs
  15. 1 0
      Source/QuestPDF/Skia/SkBitmap.cs
  16. 1 0
      Source/QuestPDF/Skia/SkCanvas.cs
  17. 1 0
      Source/QuestPDF/Skia/SkData.cs
  18. 1 0
      Source/QuestPDF/Skia/SkDocument.cs
  19. 1 0
      Source/QuestPDF/Skia/SkImage.cs
  20. 1 0
      Source/QuestPDF/Skia/SkPicture.cs
  21. 17 0
      Source/QuestPDF/Skia/SkPictureRecorder.cs
  22. 1 0
      Source/QuestPDF/Skia/SkSvgImage.cs
  23. 6 4
      Source/QuestPDF/Skia/SkText.cs
  24. 1 0
      Source/QuestPDF/Skia/SkWriteStream.cs
  25. 6 0
      Source/QuestPDF/Skia/SkiaAPI.cs
  26. 1 0
      Source/QuestPDF/Skia/Text/SkFontCollection.cs
  27. 1 0
      Source/QuestPDF/Skia/Text/SkParagraph.cs
  28. 6 1
      Source/QuestPDF/Skia/Text/SkParagraphBuilder.cs
  29. 1 0
      Source/QuestPDF/Skia/Text/SkTextStyle.cs
  30. 1 0
      Source/QuestPDF/Skia/Text/SkTypeface.cs
  31. 7 0
      Source/QuestPDF/Skia/Text/SkTypefaceProvider.cs

+ 1 - 0
Source/QuestPDF/Drawing/CompanionCanvas.cs

@@ -47,6 +47,7 @@ namespace QuestPDF.Drawing
         
         ~CompanionCanvas()
         {
+            this.WarnThatFinalizerIsReached();
             Dispose();
         }
 

+ 1 - 0
Source/QuestPDF/Drawing/ImageCanvas.cs

@@ -22,6 +22,7 @@ namespace QuestPDF.Drawing
         
         ~ImageCanvas()
         {
+            this.WarnThatFinalizerIsReached();
             Dispose();
         }
 

+ 1 - 0
Source/QuestPDF/Drawing/Proxy/SnapshotRecorder.cs

@@ -14,6 +14,7 @@ internal class SnapshotRecorder : ElementProxy, IDisposable
 
     ~SnapshotRecorder()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
 

+ 1 - 0
Source/QuestPDF/Drawing/SkiaDocumentCanvasBase.cs

@@ -15,6 +15,7 @@ namespace QuestPDF.Drawing
 
         ~SkiaDocumentCanvasBase()
         {
+            this.WarnThatFinalizerIsReached();
             Dispose();
         }
 

+ 1 - 3
Source/QuestPDF/Drawing/SvgCanvas.cs

@@ -1,9 +1,6 @@
 using System;
 using System.Collections.Generic;
-using System.IO;
 using System.Text;
-using QuestPDF.Drawing.Exceptions;
-using QuestPDF.Helpers;
 using QuestPDF.Infrastructure;
 using QuestPDF.Skia;
 
@@ -16,6 +13,7 @@ namespace QuestPDF.Drawing
         
         ~SvgCanvas()
         {
+            this.WarnThatFinalizerIsReached();
             Dispose();
         }
 

+ 1 - 0
Source/QuestPDF/Elements/DynamicImage.cs

@@ -38,6 +38,7 @@ namespace QuestPDF.Elements
         
         ~DynamicImage()
         {
+            this.WarnThatFinalizerIsReached();
             Dispose();
         }
 

+ 1 - 0
Source/QuestPDF/Elements/DynamicSvgImage.cs

@@ -17,6 +17,7 @@ internal class DynamicSvgImage : Element, IStateful, IDisposable
     
     ~DynamicSvgImage()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 9 - 3
Source/QuestPDF/Elements/Image.cs

@@ -15,13 +15,19 @@ namespace QuestPDF.Elements
         internal ImageCompressionQuality? CompressionQuality { get; set; }
  
         private int DrawnImageSize { get; set; }
+
+        ~Image()
+        {
+            this.WarnThatFinalizerIsReached();
+            Dispose();
+        }
         
         public void Dispose()
         {
-            if (DocumentImage == null || DocumentImage.IsShared)
-                return;
+            if (DocumentImage != null && !DocumentImage.IsShared) 
+                DocumentImage?.Dispose();
             
-            DocumentImage?.Dispose();
+            GC.SuppressFinalize(this);
         }
         
         internal override SpacePlan Measure(Size availableSpace)

+ 10 - 4
Source/QuestPDF/Elements/SvgImage.cs

@@ -10,12 +10,18 @@ internal class SvgImage : Element, IStateful, IDisposable
 {
     public Infrastructure.SvgImage Image { get; set; }
     
+    ~SvgImage()
+    {
+        this.WarnThatFinalizerIsReached();
+        Dispose();
+    }
+    
     public void Dispose()
     {
-        if (Image == null || Image.IsShared)
-            return;
-            
-        Image?.Dispose();
+        if (Image != null && !Image.IsShared)
+            Image?.Dispose();
+        
+        GC.SuppressFinalize(this);
     }
     
     internal override SpacePlan Measure(Size availableSpace)

+ 7 - 0
Source/QuestPDF/Elements/Text/TextBlock.cs

@@ -48,6 +48,7 @@ namespace QuestPDF.Elements.Text
 
         ~TextBlock()
         {
+            this.WarnThatFinalizerIsReached();
             Dispose();
         }
 
@@ -290,6 +291,12 @@ namespace QuestPDF.Elements.Text
                 MaxLinesVisible = LineClamp ?? 1_000_000,
                 LineClampEllipsis = LineClampEllipsis
             };
+
+            if (Paragraph != null)
+            {
+                Paragraph.Dispose();
+                Paragraph = null;
+            }
             
             var builder = SkParagraphBuilderPoolManager.Get(paragraphStyle);
 

+ 1 - 0
Source/QuestPDF/Infrastructure/Image.cs

@@ -43,6 +43,7 @@ namespace QuestPDF.Infrastructure
         
         ~Image()
         {
+            this.WarnThatFinalizerIsReached();
             Dispose();
         }
         

+ 1 - 0
Source/QuestPDF/Infrastructure/SvgImage.cs

@@ -24,6 +24,7 @@ public class SvgImage : IDisposable
 
     ~SvgImage()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
         

+ 25 - 4
Source/QuestPDF/Infrastructure/TextStyle.cs

@@ -34,6 +34,13 @@ namespace QuestPDF.Infrastructure
         internal float? DecorationThickness { get; set; }
         internal TextDirection? Direction { get; set; }
 
+        ~TextStyle()
+        {
+            // TextStyle is meant to be an object spanning the entire application lifetime
+            // It does not require the IDisposable pattern
+            SkTextStyleCache?.Dispose();
+        }
+
         public static TextStyle Default { get; } = new()
         {
             Id = 0
@@ -69,16 +76,30 @@ namespace QuestPDF.Infrastructure
             LineHeight = 1
         };
 
-        private SkTextStyle? SkTextStyleCache;
+        private volatile SkTextStyle? SkTextStyleCache;
+        private readonly object SkTextStyleCacheLock = new();
         
         internal SkTextStyle GetSkTextStyle()
         {
             if (SkTextStyleCache != null)
                 return SkTextStyleCache;
-            
+
+            lock (SkTextStyleCacheLock)
+            {
+                if (SkTextStyleCache != null)
+                    return SkTextStyleCache;
+
+                var temp = CreateSkTextStyle();
+                SkTextStyleCache = temp;
+                return temp;
+            }
+        }
+
+        private SkTextStyle CreateSkTextStyle()
+        {
             var fontFamilyTexts = FontFamilies.Select(x => new SkText(x)).ToList();
 
-            SkTextStyleCache = new SkTextStyle(new TextStyleConfiguration
+            var result = new SkTextStyle(new TextStyleConfiguration
             {
                 FontSize = CalculateTargetFontSize(),
                 FontWeight = (TextStyleConfiguration.FontWeights?)FontWeight ?? TextStyleConfiguration.FontWeights.Normal,
@@ -103,7 +124,7 @@ namespace QuestPDF.Infrastructure
             });
             
             fontFamilyTexts.ForEach(x => x.Dispose());
-            return SkTextStyleCache;
+            return result;
 
             IntPtr[] GetFontFamilyPointers(IList<SkText> texts)
             {

+ 6 - 9
Source/QuestPDF/Infrastructure/TextStyleManager.cs

@@ -135,7 +135,6 @@ namespace QuestPDF.Infrastructure
                 
                 var newIndex = TextStyles.Count;
                 var newTextStyle = origin with { Id = newIndex };
-                newTextStyle.Id = newIndex;
                 property.SetValue(newTextStyle, newValue);
 
                 TextStyles.Add(newTextStyle);
@@ -150,16 +149,15 @@ namespace QuestPDF.Infrastructure
                 if (overrideValue && newValue is null)
                     return origin;
                 
-                var newIndex = TextStyles.Count;
-                var newTextStyle = origin with { Id = newIndex };
-                newTextStyle.Id = newIndex;
-                
                 newValue ??= Array.Empty<string>();
                 var oldValue = origin.FontFamilies ?? Array.Empty<string>();
                 
                 if (origin.FontFamilies?.SequenceEqual(newValue) == true)
                     return origin;
                 
+                var newIndex = TextStyles.Count;
+                var newTextStyle = origin with { Id = newIndex };
+                
                 newTextStyle.FontFamilies = overrideValue 
                     ? newValue 
                     : oldValue.Concat(newValue).Where(x => !string.IsNullOrEmpty(x)).Distinct().ToArray();
@@ -176,10 +174,6 @@ namespace QuestPDF.Infrastructure
                 if (overrideValue && newValue is null)
                     return origin;
                 
-                var newIndex = TextStyles.Count;
-                var newTextStyle = origin with { Id = newIndex };
-                newTextStyle.Id = newIndex;
-                
                 newValue ??= [];
                 var oldValue = origin.FontFeatures ?? [];
                 
@@ -190,6 +184,9 @@ namespace QuestPDF.Infrastructure
                     ? newValue.Concat(oldValue)
                     : oldValue.Concat(newValue);
                 
+                var newIndex = TextStyles.Count;
+                var newTextStyle = origin with { Id = newIndex };
+                
                 newTextStyle.FontFeatures = extendedSet
                     .GroupBy(x => x.Name)
                     .Select(x => x.First())

+ 1 - 0
Source/QuestPDF/Skia/SkBitmap.cs

@@ -33,6 +33,7 @@ internal sealed class SkBitmap : IDisposable
     
     ~SkBitmap()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 1 - 0
Source/QuestPDF/Skia/SkCanvas.cs

@@ -123,6 +123,7 @@ internal sealed class SkCanvas : IDisposable
     
     ~SkCanvas()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 1 - 0
Source/QuestPDF/Skia/SkData.cs

@@ -58,6 +58,7 @@ internal sealed class SkData : IDisposable
     
     ~SkData()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 1 - 0
Source/QuestPDF/Skia/SkDocument.cs

@@ -31,6 +31,7 @@ internal sealed class SkDocument : IDisposable
     
     ~SkDocument()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 1 - 0
Source/QuestPDF/Skia/SkImage.cs

@@ -61,6 +61,7 @@ internal sealed class SkImage : IDisposable
     
     ~SkImage()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 1 - 0
Source/QuestPDF/Skia/SkPicture.cs

@@ -27,6 +27,7 @@ internal sealed class SkPicture : IDisposable
     
     ~SkPicture()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 17 - 0
Source/QuestPDF/Skia/SkPictureRecorder.cs

@@ -6,6 +6,7 @@ namespace QuestPDF.Skia;
 internal sealed class SkPictureRecorder : IDisposable
 {
     public IntPtr Instance { get; private set; }
+    private bool IsRecording { get; set; }
     
     public SkPictureRecorder()
     {
@@ -16,17 +17,20 @@ internal sealed class SkPictureRecorder : IDisposable
     public SkCanvas BeginRecording(float width, float height)
     {
         var canvasInstance = API.picture_recorder_begin_recording(Instance, width, height);
+        IsRecording = true;
         return new SkCanvas(canvasInstance, disposeNativeObject: false);
     }
     
     public SkPicture EndRecording()
     {
         var pictureInstance = API.picture_recorder_end_recording(Instance);
+        IsRecording = false;
         return new SkPicture(pictureInstance);
     }
     
     ~SkPictureRecorder()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     
@@ -34,6 +38,19 @@ internal sealed class SkPictureRecorder : IDisposable
     {
         if (Instance == IntPtr.Zero)
             return;
+
+        if (IsRecording)
+        {
+            try
+            {
+                var picture = EndRecording();
+                picture.Dispose();
+            }
+            catch
+            {
+                // ignored
+            }
+        }
         
         API.picture_recorder_delete(Instance);
         Instance = IntPtr.Zero;

+ 1 - 0
Source/QuestPDF/Skia/SkSvgImage.cs

@@ -58,6 +58,7 @@ internal sealed class SkSvgImage : IDisposable
     
     ~SkSvgImage()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 6 - 4
Source/QuestPDF/Skia/SkText.cs

@@ -15,6 +15,7 @@ internal class SkText : IDisposable
 
     ~SkText()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     
@@ -41,11 +42,12 @@ internal class SkText : IDisposable
     
     public void Dispose()
     {
-        if (Instance == IntPtr.Zero)
-            return;
+        if (Instance != IntPtr.Zero)
+        {
+            Marshal.FreeHGlobal(Instance);
+            Instance = IntPtr.Zero;
+        }
         
-        Marshal.FreeHGlobal(Instance);
-        Instance = IntPtr.Zero;
         GC.SuppressFinalize(this);
     }
 }

+ 1 - 0
Source/QuestPDF/Skia/SkWriteStream.cs

@@ -21,6 +21,7 @@ internal sealed class SkWriteStream : IDisposable
     
     ~SkWriteStream()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 6 - 0
Source/QuestPDF/Skia/SkiaAPI.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Diagnostics;
 using QuestPDF.Drawing.Exceptions;
 
 namespace QuestPDF.Skia;
@@ -12,4 +13,9 @@ internal static class SkiaAPI
         if (instance == IntPtr.Zero)
             throw new InitializationException($"QuestPDF cannot instantiate native object.");
     }
+    
+    public static void WarnThatFinalizerIsReached<T>(this T disposableObject) where T : IDisposable
+    {
+        Debug.Fail($"An object of type '{typeof(T).Name}' was not disposed explicitly, and was finalized instead.");
+    }
 }

+ 1 - 0
Source/QuestPDF/Skia/Text/SkFontCollection.cs

@@ -21,6 +21,7 @@ internal sealed class SkFontCollection : IDisposable
     
     ~SkFontCollection()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 1 - 0
Source/QuestPDF/Skia/Text/SkParagraph.cs

@@ -85,6 +85,7 @@ internal sealed class SkParagraph : IDisposable
     
     ~SkParagraph()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 6 - 1
Source/QuestPDF/Skia/Text/SkParagraphBuilder.cs

@@ -95,6 +95,7 @@ internal sealed class SkParagraphBuilder : IDisposable
     public IntPtr Instance { get; private set; }
     
     public ParagraphStyle Style { get; private set; }
+    private SkFontCollection FontCollection { get; set; }
 
     public static SkParagraphBuilder Create(ParagraphStyle style, SkFontCollection fontCollection)
     {
@@ -114,7 +115,8 @@ internal sealed class SkParagraphBuilder : IDisposable
         return new SkParagraphBuilder
         {
             Instance = instance,
-            Style = style
+            Style = style,
+            FontCollection = fontCollection
         };
     }
     
@@ -141,6 +143,7 @@ internal sealed class SkParagraphBuilder : IDisposable
     
     ~SkParagraphBuilder()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     
@@ -149,6 +152,8 @@ internal sealed class SkParagraphBuilder : IDisposable
         if (Instance == IntPtr.Zero)
             return;
         
+        FontCollection?.Dispose();
+        
         API.paragraph_builder_delete(Instance);
         Instance = IntPtr.Zero;
         GC.SuppressFinalize(this);

+ 1 - 0
Source/QuestPDF/Skia/Text/SkTextStyle.cs

@@ -89,6 +89,7 @@ internal sealed class SkTextStyle : IDisposable
     
     ~SkTextStyle()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 1 - 0
Source/QuestPDF/Skia/Text/SkTypeface.cs

@@ -15,6 +15,7 @@ internal sealed class SkTypeface : IDisposable
     
     ~SkTypeface()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     

+ 7 - 0
Source/QuestPDF/Skia/Text/SkTypefaceProvider.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Runtime.InteropServices;
 
 namespace QuestPDF.Skia.Text;
@@ -6,6 +7,7 @@ namespace QuestPDF.Skia.Text;
 internal sealed class SkTypefaceProvider : IDisposable
 {
     public IntPtr Instance { get; private set; }
+    private List<SkTypeface> Typefaces { get; } = new();
     
     public SkTypefaceProvider()
     {
@@ -16,6 +18,7 @@ internal sealed class SkTypefaceProvider : IDisposable
     public void AddTypefaceFromData(SkData data, string? alias = null)
     {
         var typeface = SkFontManager.Global.CreateTypeface(data);
+        Typefaces.Add(typeface);
         
         if (alias == null)
             API.typeface_font_provider_add_typeface(Instance, typeface.Instance);
@@ -25,6 +28,7 @@ internal sealed class SkTypefaceProvider : IDisposable
     
     ~SkTypefaceProvider()
     {
+        this.WarnThatFinalizerIsReached();
         Dispose();
     }
     
@@ -33,6 +37,9 @@ internal sealed class SkTypefaceProvider : IDisposable
         if (Instance == IntPtr.Zero)
             return;
         
+        foreach (var typeface in Typefaces)
+            typeface.Dispose();
+        
         API.typeface_font_provider_unref(Instance);
         Instance = IntPtr.Zero;
         GC.SuppressFinalize(this);