Skip to content
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

Fix IDisposable usage #411

Merged
merged 2 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/SixLabors.Fonts/Buffer{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, didn't get the idea about the purpose of private field. You can contribute to my repo if you wish.

this.Memory = manager.Memory.Slice(0, this.length);
this.span = this.Memory.Span;

Expand Down
2 changes: 1 addition & 1 deletion src/SixLabors.Fonts/FileFontMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public static FileFontMetrics[] LoadFontCollection(string path)
{
using FileStream fs = File.OpenRead(path);
long startPos = fs.Position;
BigEndianBinaryReader reader = new(fs, true);
using BigEndianBinaryReader reader = new(fs, true);
TtcHeader ttcHeader = TtcHeader.Read(reader);
FileFontMetrics[] fonts = new FileFontMetrics[(int)ttcHeader.NumFonts];

Expand Down
2 changes: 1 addition & 1 deletion src/SixLabors.Fonts/FontCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ private IEnumerable<FontFamily> AddCollectionImpl(
out IEnumerable<FontDescription> descriptions)
{
long startPos = stream.Position;
BigEndianBinaryReader reader = new(stream, true);
using BigEndianBinaryReader reader = new(stream, true);
TtcHeader ttcHeader = TtcHeader.Read(reader);
List<FontDescription> result = new((int)ttcHeader.NumFonts);
HashSet<FontFamily> installedFamilies = new();
Expand Down
8 changes: 4 additions & 4 deletions src/SixLabors.Fonts/FontDescription.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public static FontDescription LoadDescription(string path)
Guard.NotNullOrWhiteSpace(path, nameof(path));

using FileStream fs = File.OpenRead(path);
var reader = new FontReader(fs);
using var reader = new FontReader(fs);
return LoadDescription(reader);
}

Expand All @@ -106,7 +106,7 @@ public static FontDescription LoadDescription(Stream stream)
Guard.NotNull(stream, nameof(stream));

// Only read the name tables.
var reader = new FontReader(stream);
using var reader = new FontReader(stream);

return LoadDescription(reader);
}
Expand Down Expand Up @@ -152,14 +152,14 @@ public static FontDescription[] LoadFontCollectionDescriptions(string path)
public static FontDescription[] LoadFontCollectionDescriptions(Stream stream)
{
long startPos = stream.Position;
var reader = new BigEndianBinaryReader(stream, true);
using var reader = new BigEndianBinaryReader(stream, true);
var ttcHeader = TtcHeader.Read(reader);

var result = new FontDescription[(int)ttcHeader.NumFonts];
for (int i = 0; i < ttcHeader.NumFonts; ++i)
{
stream.Position = startPos + ttcHeader.OffsetTable[i];
var fontReader = new FontReader(stream);
using var fontReader = new FontReader(stream);
result[i] = LoadDescription(fontReader);
}

Expand Down
25 changes: 21 additions & 4 deletions src/SixLabors.Fonts/FontReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,23 @@

namespace SixLabors.Fonts;

internal sealed class FontReader
internal sealed class FontReader : IDisposable
{
private readonly Stream stream;
private readonly Dictionary<Type, Table> loadedTables = new();
private readonly TableLoader loader;

private readonly bool isOwnedStream;
private bool isDisposed;

internal FontReader(Stream stream, TableLoader loader)
{
this.loader = loader;

Func<BigEndianBinaryReader, TableHeader> loadHeader = TableHeader.Read;
long startOfFilePosition = stream.Position;

this.stream = stream;
var reader = new BigEndianBinaryReader(stream, true);
using var reader = new BigEndianBinaryReader(stream, true);

// we should immediately read the table header to learn which tables we have and what order they are in
uint version = reader.ReadUInt32();
Expand Down Expand Up @@ -85,13 +87,14 @@ internal FontReader(Stream stream, TableLoader loader)
this.CompressedTableData = true;
this.Headers = Woff2Utils.ReadWoff2Headers(reader, tableCount);

this.isOwnedStream = true;

byte[] compressedBuffer = reader.ReadBytes((int)totalCompressedSize);
var decompressedStream = new MemoryStream();
using var input = new MemoryStream(compressedBuffer);
using var decompressor = new BrotliStream(input, CompressionMode.Decompress);
decompressor.CopyTo(decompressedStream);
decompressedStream.Position = 0;
this.stream.Dispose();
Bykiev marked this conversation as resolved.
Show resolved Hide resolved
this.stream = decompressedStream;
return;
}
Expand Down Expand Up @@ -197,4 +200,18 @@ public bool TryGetReaderAtTablePosition(string tableName, [NotNullWhen(returnVal
reader = header?.CreateReader(this.stream);
return reader != null;
}

public void Dispose()
{
if (this.isDisposed)
{
return;
}

if (this.isOwnedStream)
{
this.stream.Dispose();
this.isDisposed = true;
}
}
}
2 changes: 1 addition & 1 deletion src/SixLabors.Fonts/Native/MacSystemFontsEnumerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace SixLabors.Fonts.Native;
/// Internally, it calls the native CoreText's <see cref="CTFontManagerCopyAvailableFontURLs"/> method to retrieve
/// the list of fonts so using this class must be guarded by <c>RuntimeInformation.IsOSPlatform(OSPlatform.OSX)</c>.
/// </remarks>
internal class MacSystemFontsEnumerator : IEnumerable<string>, IEnumerator<string>
internal sealed class MacSystemFontsEnumerator : IEnumerable<string>, IEnumerator<string>
{
private static readonly ArrayPool<byte> BytePool = ArrayPool<byte>.Shared;

Expand Down
4 changes: 2 additions & 2 deletions src/SixLabors.Fonts/StreamFontMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ internal override void UpdatePositions(GlyphPositioningCollection collection)
public static StreamFontMetrics LoadFont(string path)
{
using FileStream fs = File.OpenRead(path);
var reader = new FontReader(fs);
using var reader = new FontReader(fs);
return LoadFont(reader);
}

Expand All @@ -357,7 +357,7 @@ public static StreamFontMetrics LoadFont(string path, long offset)
/// <returns>a <see cref="StreamFontMetrics"/>.</returns>
public static StreamFontMetrics LoadFont(Stream stream)
{
var reader = new FontReader(stream);
using var reader = new FontReader(stream);
return LoadFont(reader);
}

Expand Down
24 changes: 12 additions & 12 deletions tests/SixLabors.Fonts.Tests/FontReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public void ReadTrueTypeOutlineType()
var writer = new BigEndianBinaryWriter();
writer.WriteTrueTypeFileHeader(0, 0, 0, 0);

var reader = new FontReader(writer.GetStream());
using var reader = new FontReader(writer.GetStream());
Assert.Equal(OutlineType.TrueType, reader.OutlineType);
}

Expand All @@ -25,7 +25,7 @@ public void ReadCffOutlineType()
var writer = new BigEndianBinaryWriter();
writer.WriteCffFileHeader(0, 0, 0, 0);

var reader = new FontReader(writer.GetStream());
using var reader = new FontReader(writer.GetStream());
Assert.Equal(OutlineType.CFF, reader.OutlineType);
}

Expand All @@ -37,7 +37,7 @@ public void ReadTableHeaders()
writer.WriteTableHeader("name", 0, 10, 0);
writer.WriteTableHeader("cmap", 0, 1, 0);

var reader = new FontReader(writer.GetStream());
using var reader = new FontReader(writer.GetStream());

Assert.Equal(2, reader.Headers.Count);
}
Expand All @@ -59,16 +59,16 @@ public void ReadCMapTable()
new byte[] { 2, 9 })
});

var reader = new FontReader(writer.GetStream());
using var reader = new FontReader(writer.GetStream());
CMapTable cmap = reader.GetTable<CMapTable>();
Assert.NotNull(cmap);
}

[Fact]
public void ReadFont_WithWoffFormat_EqualsTtf()
{
var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff1Data());
using var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
using var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff1Data());

Assert.Equal(fontReaderTtf.Headers.Count, fontReaderWoff.Headers.Count);
foreach (string key in fontReaderTtf.Headers.Keys)
Expand All @@ -80,9 +80,9 @@ public void ReadFont_WithWoffFormat_EqualsTtf()
[Fact]
public void GlyphsCount_WithWoffFormat_EqualsTtf()
{
var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff1Data());
using var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff1Data());
GlyphTable glyphsWoff = fontReaderWoff.GetTable<GlyphTable>();
var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
using var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
GlyphTable glyphsTtf = fontReaderTtf.GetTable<GlyphTable>();

Assert.Equal(glyphsTtf.GlyphCount, glyphsWoff.GlyphCount);
Expand All @@ -91,8 +91,8 @@ public void GlyphsCount_WithWoffFormat_EqualsTtf()
[Fact]
public void ReadFont_WithWoff2Format_EqualsTtf()
{
var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff2Data());
using var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
using var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff2Data());

Assert.Equal(fontReaderTtf.Headers.Count, fontReaderWoff.Headers.Count);
foreach (string key in fontReaderTtf.Headers.Keys)
Expand All @@ -104,9 +104,9 @@ public void ReadFont_WithWoff2Format_EqualsTtf()
[Fact]
public void GlyphsCount_WithWoff2Format_EqualsTtf()
{
var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff2Data());
using var fontReaderWoff = new FontReader(TestFonts.OpensSansWoff2Data());
GlyphTable glyphsWoff = fontReaderWoff.GetTable<GlyphTable>();
var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
using var fontReaderTtf = new FontReader(TestFonts.OpenSansTtfData());
GlyphTable glyphsTtf = fontReaderTtf.GetTable<GlyphTable>();

Assert.Equal(glyphsTtf.GlyphCount, glyphsWoff.GlyphCount);
Expand Down
6 changes: 5 additions & 1 deletion tests/SixLabors.Fonts.Tests/Tables/General/CMapTableTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ public void ShouldThrowExceptionWhenTableCouldNotBeFound()

using (System.IO.MemoryStream stream = writer.GetStream())
{
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(() => CMapTable.Load(new FontReader(stream)));
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(() =>
{
using var reader = new FontReader(stream);
CMapTable.Load(reader);
});

Assert.Equal("cmap", exception.Table);
}
Expand Down
11 changes: 6 additions & 5 deletions tests/SixLabors.Fonts.Tests/Tables/General/ColrTableTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ public void ShouldReturnNullWhenTableCouldNotBeFound()
var writer = new BigEndianBinaryWriter();
writer.WriteTrueTypeFileHeader();

using (System.IO.MemoryStream stream = writer.GetStream())
using (MemoryStream stream = writer.GetStream())
{
Assert.Null(ColrTable.Load(new FontReader(stream)));
using var reader = new FontReader(stream);
Assert.Null(ColrTable.Load(reader));
}
}

Expand Down Expand Up @@ -47,12 +48,12 @@ public void ShouldReturnTableValues()
}
});

using (System.IO.Stream stream = TestFonts.TwemojiMozillaData())
using (Stream stream = TestFonts.TwemojiMozillaData())
{
var reader = new FontReader(stream);
using var reader = new FontReader(stream);
ColrTable tbl = reader.GetTable<ColrTable>();

System.Span<LayerRecord> layers = tbl.GetLayers(15);
Span<LayerRecord> layers = tbl.GetLayers(15);
Assert.Equal(2, layers.Length);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ public void ShouldReturnNullWhenTableCouldNotBeFound()
var writer = new BigEndianBinaryWriter();
writer.WriteTrueTypeFileHeader();

using (System.IO.MemoryStream stream = writer.GetStream())
using (MemoryStream stream = writer.GetStream())
{
Assert.Null(HorizontalHeadTable.Load(new FontReader(stream)));
using var reader = new FontReader(stream);
Assert.Null(HorizontalHeadTable.Load(reader));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@ public void ShouldThrowExceptionWhenHeadTableCouldNotBeFound()
var writer = new BigEndianBinaryWriter();
writer.WriteTrueTypeFileHeader();

using (System.IO.MemoryStream stream = writer.GetStream())
using (MemoryStream stream = writer.GetStream())
{
MissingFontTableException exception = Assert.Throws<MissingFontTableException>(
() => IndexLocationTable.Load(new FontReader(stream)));
MissingFontTableException exception = Assert.Throws<MissingFontTableException>(() =>
{
using var reader = new FontReader(stream);
IndexLocationTable.Load(reader);
});

Assert.Equal("head", exception.Table);
}
Expand All @@ -40,10 +43,13 @@ public void ShouldThrowExceptionWhenMaximumProfileTableCouldNotBeFound()
0,
HeadTable.IndexLocationFormats.Offset16));

using (System.IO.MemoryStream stream = writer.GetStream())
using (MemoryStream stream = writer.GetStream())
{
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(
() => IndexLocationTable.Load(new FontReader(stream)));
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(() =>
{
using var reader = new FontReader(stream);
IndexLocationTable.Load(reader);
});

Assert.Equal("maxp", exception.Table);
}
Expand All @@ -65,9 +71,10 @@ public void ShouldReturnNullWhenTableCouldNotBeFound()
0,
HeadTable.IndexLocationFormats.Offset16));

using (System.IO.MemoryStream stream = writer.GetStream())
using (MemoryStream stream = writer.GetStream())
{
Assert.Null(IndexLocationTable.Load(new FontReader(stream)));
using var reader = new FontReader(stream);
Assert.Null(IndexLocationTable.Load(reader));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Six Labors.
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

using SixLabors.Fonts.Tables.General.Kern;
Expand All @@ -13,9 +13,10 @@ public void ShouldReturnDefaultValueWhenTableCouldNotBeFound()
var writer = new BigEndianBinaryWriter();
writer.WriteTrueTypeFileHeader();

using (System.IO.MemoryStream stream = writer.GetStream())
using (MemoryStream stream = writer.GetStream())
{
var table = KerningTable.Load(new FontReader(stream));
using var reader = new FontReader(stream);
var table = KerningTable.Load(reader);
Assert.NotNull(table);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ public void ShouldThrowExceptionWhenTableCouldNotBeFound()

using (MemoryStream stream = writer.GetStream())
{
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(
() => MaximumProfileTable.Load(new FontReader(stream)));
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(() =>
{
using var reader = new FontReader(stream);
MaximumProfileTable.Load(reader);
});

Assert.Equal("maxp", exception.Table);
}
Expand Down
9 changes: 6 additions & 3 deletions tests/SixLabors.Fonts.Tests/Tables/General/NameTableTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,13 @@ public void ShouldThrowExceptionWhenTableCouldNotBeFound()
var writer = new BigEndianBinaryWriter();
writer.WriteTrueTypeFileHeader();

using (System.IO.MemoryStream stream = writer.GetStream())
using (MemoryStream stream = writer.GetStream())
{
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(
() => NameTable.Load(new FontReader(stream)));
InvalidFontTableException exception = Assert.Throws<InvalidFontTableException>(() =>
{
using var reader = new FontReader(stream);
NameTable.Load(reader);
});

Assert.Equal("name", exception.Table);
}
Expand Down
Loading
Loading