Skip to content

Commit 4a32c75

Browse files
JimBobSquarePantsJimBobSquarePants
authored
Merge branch 'main' into js/decode-sanitation
2 parents ced9887 + 7db4792 commit 4a32c75

23 files changed

+259
-66
lines changed

‎src/Directory.Build.props‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
<InternalsVisibleTo Include="ImageSharp.Benchmarks" Key="$(SixLaborsPublicKey)" />
2828
<InternalsVisibleTo Include="ImageSharp.Tests.ProfilingSandbox" Key="$(SixLaborsPublicKey)" />
2929
<InternalsVisibleTo Include="SixLabors.ImageSharp.Tests" Key="$(SixLaborsPublicKey)" />
30+
<InternalsVisibleTo Include="SixLabors.ImageSharp.Drawing.Tests" Key="$(SixLaborsPublicKey)" />
3031
</ItemGroup>
3132

3233
</Project>

‎src/ImageSharp/Diagnostics/MemoryDiagnostics.cs‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Six Labors.
22
// Licensed under the Apache License, Version 2.0.
33

4+
using System;
45
using System.Threading;
56

67
namespace SixLabors.ImageSharp.Diagnostics
@@ -47,18 +48,34 @@ public static event UndisposedAllocationDelegate UndisposedAllocation
4748
}
4849
}
4950

51+
/// <summary>
52+
/// Fires when ImageSharp allocates memory from a MemoryAllocator
53+
/// </summary>
54+
internal static event Action MemoryAllocated;
55+
56+
/// <summary>
57+
/// Fires when ImageSharp releases memory allocated from a MemoryAllocator
58+
/// </summary>
59+
internal static event Action MemoryReleased;
60+
5061
/// <summary>
5162
/// Gets a value indicating the total number of memory resource objects leaked to the finalizer.
5263
/// </summary>
5364
public static int TotalUndisposedAllocationCount => totalUndisposedAllocationCount;
5465

5566
internal static bool UndisposedAllocationSubscribed => Volatile.Read(ref undisposedAllocationSubscriptionCounter) > 0;
5667

57-
internal static void IncrementTotalUndisposedAllocationCount() =>
68+
internal static void IncrementTotalUndisposedAllocationCount()
69+
{
5870
Interlocked.Increment(ref totalUndisposedAllocationCount);
71+
MemoryAllocated?.Invoke();
72+
}
5973

60-
internal static void DecrementTotalUndisposedAllocationCount() =>
74+
internal static void DecrementTotalUndisposedAllocationCount()
75+
{
6176
Interlocked.Decrement(ref totalUndisposedAllocationCount);
77+
MemoryReleased?.Invoke();
78+
}
6279

6380
internal static void RaiseUndisposedMemoryResource(string allocationStackTrace)
6481
{

‎src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,12 @@ public BmpDecoderCore(Configuration configuration, IBmpDecoderOptions options)
122122
public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken cancellationToken)
123123
where TPixel : unmanaged, IPixel<TPixel>
124124
{
125+
Image<TPixel> image = null;
125126
try
126127
{
127128
int bytesPerColorMapEntry = this.ReadImageHeaders(stream, out bool inverted, out byte[] palette);
128129

129-
var image = new Image<TPixel>(this.Configuration, this.infoHeader.Width, this.infoHeader.Height, this.metadata);
130+
image = new Image<TPixel>(this.Configuration, this.infoHeader.Width, this.infoHeader.Height, this.metadata);
130131

131132
Buffer2D<TPixel> pixels = image.GetRootFramePixelBuffer();
132133

@@ -193,8 +194,14 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
193194
}
194195
catch (IndexOutOfRangeException e)
195196
{
197+
image?.Dispose();
196198
throw new ImageFormatException("Bitmap does not have a valid format.", e);
197199
}
200+
catch
201+
{
202+
image?.Dispose();
203+
throw;
204+
}
198205
}
199206

200207
/// <inheritdoc />

‎src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ public Buffer2D<TPixel> GetPixelBuffer(CancellationToken cancellationToken)
9595
}
9696
}
9797

98-
return this.pixelBuffer;
98+
var buffer = this.pixelBuffer;
99+
this.pixelBuffer = null;
100+
return buffer;
99101
}
100102

101103
/// <inheritdoc/>
@@ -210,6 +212,7 @@ public void Dispose()
210212

211213
this.rgbBuffer?.Dispose();
212214
this.paddedProxyPixelRow?.Dispose();
215+
this.pixelBuffer?.Dispose();
213216
}
214217
}
215218
}

‎src/ImageSharp/Formats/Png/PngDecoderCore.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,16 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
227227

228228
return image;
229229
}
230+
catch
231+
{
232+
image?.Dispose();
233+
throw;
234+
}
230235
finally
231236
{
232237
this.scanline?.Dispose();
233238
this.previousScanline?.Dispose();
239+
this.nextChunk?.Data?.Dispose();
234240
}
235241
}
236242

@@ -472,6 +478,8 @@ private void InitializeImage<TPixel>(ImageMetadata metadata, out Image<TPixel> i
472478
this.bytesPerSample = this.header.BitDepth / 8;
473479
}
474480

481+
this.previousScanline?.Dispose();
482+
this.scanline?.Dispose();
475483
this.previousScanline = this.memoryAllocator.Allocate<byte>(this.bytesPerScanline, AllocationOptions.Clean);
476484
this.scanline = this.Configuration.MemoryAllocator.Allocate<byte>(this.bytesPerScanline, AllocationOptions.Clean);
477485
}
@@ -1359,6 +1367,7 @@ private int ReadNextDataChunk()
13591367
{
13601368
if (chunk.Type == PngChunkType.Data)
13611369
{
1370+
chunk.Data?.Dispose();
13621371
return chunk.Length;
13631372
}
13641373

@@ -1453,6 +1462,9 @@ private void ValidateChunk(in PngChunk chunk)
14531462
if (validCrc != inputCrc)
14541463
{
14551464
string chunkTypeName = Encoding.ASCII.GetString(chunkType);
1465+
1466+
// ensure when throwing we dispose the data back to the memory allocator
1467+
chunk.Data?.Dispose();
14561468
PngThrowHelper.ThrowInvalidChunkCrc(chunkTypeName);
14571469
}
14581470
}

‎src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ protected override void Decompress(BufferedReadStream stream, int byteCount, int
6565
jpegDecoder.ParseStream(stream, scanDecoderGray, CancellationToken.None);
6666

6767
// TODO: Should we pass through the CancellationToken from the tiff decoder?
68-
CopyImageBytesToBuffer(buffer, spectralConverterGray.GetPixelBuffer(CancellationToken.None));
68+
using var decompressedBuffer = spectralConverterGray.GetPixelBuffer(CancellationToken.None);
69+
CopyImageBytesToBuffer(buffer, decompressedBuffer);
6970
break;
7071
}
7172

@@ -81,7 +82,8 @@ protected override void Decompress(BufferedReadStream stream, int byteCount, int
8182
jpegDecoder.ParseStream(stream, scanDecoder, CancellationToken.None);
8283

8384
// TODO: Should we pass through the CancellationToken from the tiff decoder?
84-
CopyImageBytesToBuffer(buffer, spectralConverter.GetPixelBuffer(CancellationToken.None));
85+
using var decompressedBuffer = spectralConverter.GetPixelBuffer(CancellationToken.None);
86+
CopyImageBytesToBuffer(buffer, decompressedBuffer);
8587
break;
8688
}
8789

‎src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs‎

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -157,40 +157,52 @@ public TiffDecoderCore(Configuration configuration, ITiffDecoderOptions options)
157157
public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken cancellationToken)
158158
where TPixel : unmanaged, IPixel<TPixel>
159159
{
160-
this.inputStream = stream;
161-
var reader = new DirectoryReader(stream, this.Configuration.MemoryAllocator);
162-
163-
IEnumerable<ExifProfile> directories = reader.Read();
164-
this.byteOrder = reader.ByteOrder;
165-
this.isBigTiff = reader.IsBigTiff;
166-
167160
var frames = new List<ImageFrame<TPixel>>();
168-
foreach (ExifProfile ifd in directories)
161+
try
169162
{
170-
cancellationToken.ThrowIfCancellationRequested();
171-
ImageFrame<TPixel> frame = this.DecodeFrame<TPixel>(ifd, cancellationToken);
172-
frames.Add(frame);
163+
this.inputStream = stream;
164+
var reader = new DirectoryReader(stream, this.Configuration.MemoryAllocator);
165+
166+
IEnumerable<ExifProfile> directories = reader.Read();
167+
this.byteOrder = reader.ByteOrder;
168+
this.isBigTiff = reader.IsBigTiff;
173169

174-
if (this.decodingMode is FrameDecodingMode.First)
170+
foreach (ExifProfile ifd in directories)
175171
{
176-
break;
172+
cancellationToken.ThrowIfCancellationRequested();
173+
ImageFrame<TPixel> frame = this.DecodeFrame<TPixel>(ifd, cancellationToken);
174+
frames.Add(frame);
175+
176+
if (this.decodingMode is FrameDecodingMode.First)
177+
{
178+
break;
179+
}
177180
}
178-
}
179181

180-
ImageMetadata metadata = TiffDecoderMetadataCreator.Create(frames, this.ignoreMetadata, reader.ByteOrder, reader.IsBigTiff);
182+
ImageMetadata metadata = TiffDecoderMetadataCreator.Create(frames, this.ignoreMetadata, reader.ByteOrder, reader.IsBigTiff);
181183

182-
// TODO: Tiff frames can have different sizes.
183-
ImageFrame<TPixel> root = frames[0];
184-
this.Dimensions = root.Size();
185-
foreach (ImageFrame<TPixel> frame in frames)
186-
{
187-
if (frame.Size() != root.Size())
184+
// TODO: Tiff frames can have different sizes.
185+
ImageFrame<TPixel> root = frames[0];
186+
this.Dimensions = root.Size();
187+
foreach (ImageFrame<TPixel> frame in frames)
188188
{
189-
TiffThrowHelper.ThrowNotSupported("Images with different sizes are not supported");
189+
if (frame.Size() != root.Size())
190+
{
191+
TiffThrowHelper.ThrowNotSupported("Images with different sizes are not supported");
192+
}
190193
}
194+
195+
return new Image<TPixel>(this.Configuration, metadata, frames);
191196
}
197+
catch
198+
{
199+
foreach (ImageFrame<TPixel> f in frames)
200+
{
201+
f.Dispose();
202+
}
192203

193-
return new Image<TPixel>(this.Configuration, metadata, frames);
204+
throw;
205+
}
194206
}
195207

196208
/// <inheritdoc/>
@@ -240,8 +252,8 @@ private ImageFrame<TPixel> DecodeFrame<TPixel>(ExifProfile tags, CancellationTok
240252
var stripOffsetsArray = (Array)tags.GetValueInternal(ExifTag.StripOffsets).GetValue();
241253
var stripByteCountsArray = (Array)tags.GetValueInternal(ExifTag.StripByteCounts).GetValue();
242254

243-
IMemoryOwner<ulong> stripOffsetsMemory = this.ConvertNumbers(stripOffsetsArray, out Span<ulong> stripOffsets);
244-
IMemoryOwner<ulong> stripByteCountsMemory = this.ConvertNumbers(stripByteCountsArray, out Span<ulong> stripByteCounts);
255+
using IMemoryOwner<ulong> stripOffsetsMemory = this.ConvertNumbers(stripOffsetsArray, out Span<ulong> stripOffsets);
256+
using IMemoryOwner<ulong> stripByteCountsMemory = this.ConvertNumbers(stripByteCountsArray, out Span<ulong> stripByteCounts);
245257

246258
if (this.PlanarConfiguration == TiffPlanarConfiguration.Planar)
247259
{
@@ -262,8 +274,6 @@ private ImageFrame<TPixel> DecodeFrame<TPixel>(ExifProfile tags, CancellationTok
262274
cancellationToken);
263275
}
264276

265-
stripOffsetsMemory?.Dispose();
266-
stripByteCountsMemory?.Dispose();
267277
return frame;
268278
}
269279

‎src/ImageSharp/Formats/Webp/WebpDecoderCore.cs‎

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -82,38 +82,47 @@ public WebpDecoderCore(Configuration configuration, IWebpDecoderOptions options)
8282
public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken cancellationToken)
8383
where TPixel : unmanaged, IPixel<TPixel>
8484
{
85-
this.Metadata = new ImageMetadata();
86-
this.currentStream = stream;
85+
Image<TPixel> image = null;
86+
try
87+
{
88+
this.Metadata = new ImageMetadata();
89+
this.currentStream = stream;
8790

88-
uint fileSize = this.ReadImageHeader();
91+
uint fileSize = this.ReadImageHeader();
8992

90-
using (this.webImageInfo = this.ReadVp8Info())
91-
{
92-
if (this.webImageInfo.Features is { Animation: true })
93+
using (this.webImageInfo = this.ReadVp8Info())
9394
{
94-
WebpThrowHelper.ThrowNotSupportedException("Animations are not supported");
95-
}
95+
if (this.webImageInfo.Features is { Animation: true })
96+
{
97+
WebpThrowHelper.ThrowNotSupportedException("Animations are not supported");
98+
}
9699

97-
var image = new Image<TPixel>(this.Configuration, (int)this.webImageInfo.Width, (int)this.webImageInfo.Height, this.Metadata);
98-
Buffer2D<TPixel> pixels = image.GetRootFramePixelBuffer();
99-
if (this.webImageInfo.IsLossless)
100-
{
101-
var losslessDecoder = new WebpLosslessDecoder(this.webImageInfo.Vp8LBitReader, this.memoryAllocator, this.Configuration);
102-
losslessDecoder.Decode(pixels, image.Width, image.Height);
103-
}
104-
else
105-
{
106-
var lossyDecoder = new WebpLossyDecoder(this.webImageInfo.Vp8BitReader, this.memoryAllocator, this.Configuration);
107-
lossyDecoder.Decode(pixels, image.Width, image.Height, this.webImageInfo);
108-
}
100+
image = new Image<TPixel>(this.Configuration, (int)this.webImageInfo.Width, (int)this.webImageInfo.Height, this.Metadata);
101+
Buffer2D<TPixel> pixels = image.GetRootFramePixelBuffer();
102+
if (this.webImageInfo.IsLossless)
103+
{
104+
var losslessDecoder = new WebpLosslessDecoder(this.webImageInfo.Vp8LBitReader, this.memoryAllocator, this.Configuration);
105+
losslessDecoder.Decode(pixels, image.Width, image.Height);
106+
}
107+
else
108+
{
109+
var lossyDecoder = new WebpLossyDecoder(this.webImageInfo.Vp8BitReader, this.memoryAllocator, this.Configuration);
110+
lossyDecoder.Decode(pixels, image.Width, image.Height, this.webImageInfo);
111+
}
109112

110-
// There can be optional chunks after the image data, like EXIF and XMP.
111-
if (this.webImageInfo.Features != null)
112-
{
113-
this.ParseOptionalChunks(this.webImageInfo.Features);
114-
}
113+
// There can be optional chunks after the image data, like EXIF and XMP.
114+
if (this.webImageInfo.Features != null)
115+
{
116+
this.ParseOptionalChunks(this.webImageInfo.Features);
117+
}
115118

116-
return image;
119+
return image;
120+
}
121+
}
122+
catch
123+
{
124+
image?.Dispose();
125+
throw;
117126
}
118127
}
119128

‎tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
namespace SixLabors.ImageSharp.Tests.Formats.Bmp
2121
{
2222
[Trait("Format", "Bmp")]
23+
[ValidateDisposedMemoryAllocations]
2324
public class BmpDecoderTests
2425
{
2526
public const PixelTypes CommonNonDefaultPixelTypes = PixelTypes.Rgba32 | PixelTypes.Bgra32 | PixelTypes.RgbaVector;

‎tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
namespace SixLabors.ImageSharp.Tests.Formats.Gif
1919
{
2020
[Trait("Format", "Gif")]
21+
[ValidateDisposedMemoryAllocations]
2122
public class GifDecoderTests
2223
{
2324
private const PixelTypes TestPixelTypes = PixelTypes.Rgba32 | PixelTypes.RgbaVector | PixelTypes.Argb32;

0 commit comments

Comments
 (0)