Skip to content
Closed
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
1 change: 1 addition & 0 deletions binding/SkiaSharp/SKBlender.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ private sealed class SKBlenderStatic : SKBlender
internal SKBlenderStatic (IntPtr x)
: base (x, false)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
Expand Down
1 change: 1 addition & 0 deletions binding/SkiaSharp/SKColorFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ private sealed class SKColorFilterStatic : SKColorFilter
internal SKColorFilterStatic (IntPtr x)
: base (x, false)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
Expand Down
1 change: 1 addition & 0 deletions binding/SkiaSharp/SKColorSpace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ private sealed class SKColorSpaceStatic : SKColorSpace
internal SKColorSpaceStatic (IntPtr x)
: base (x, false)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
Expand Down
1 change: 1 addition & 0 deletions binding/SkiaSharp/SKData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ private sealed class SKDataStatic : SKData
internal SKDataStatic (IntPtr x)
: base (x, false)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
Expand Down
1 change: 1 addition & 0 deletions binding/SkiaSharp/SKFontManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ private sealed class SKFontManagerStatic : SKFontManager
internal SKFontManagerStatic (IntPtr x)
: base (x, false)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
Expand Down
1 change: 1 addition & 0 deletions binding/SkiaSharp/SKFontStyle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ private sealed class SKFontStyleStatic : SKFontStyle
internal SKFontStyleStatic (SKFontStyleWeight weight, SKFontStyleWidth width, SKFontStyleSlant slant)
: base (weight, width, slant)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
Expand Down
5 changes: 4 additions & 1 deletion binding/SkiaSharp/SKPaint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ public void Reset () =>

private sealed class SKFontStatic : SKFont
{
internal SKFontStatic (IntPtr handle) : base (handle, false) { }
internal SKFontStatic (IntPtr handle) : base (handle, false)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
}
Expand Down
40 changes: 31 additions & 9 deletions binding/SkiaSharp/SKTypeface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,36 @@ static SKTypeface ()

empty = new SKTypefaceStatic (SkiaApi.sk_typeface_create_empty ());

// Use legacyMakeTypeface(null) to get the platform default — this uses
// fDefaultStyleSet on Android (which searches "sans-serif", "Roboto",
// then falls back to style set 0). matchFamilyStyle(null) doesn't work
// on Android/NDK/Custom because onMatchFamily(null) returns null.
var matched = SkiaApi.sk_fontmgr_legacy_create_typeface (
SKFontManager.Default.Handle, IntPtr.Zero, SKFontStyle.Normal.Handle);
defaultTypeface = matched == IntPtr.Zero
? empty
: new SKTypefaceStatic (matched);
// Obtain the default font manager and font style as raw native handles
// instead of reading the SKFontManager.Default / SKFontStyle.Normal managed
// singletons. Those singletons are SKObject types whose static constructors
// participate in the same eager-initialization cascade (see
// SKObject.EnsureStaticInstanceAreInitialized). Reading them here can re-enter
// a static constructor that is still running on the current thread and observe
// a not-yet-assigned (null) singleton, throwing a NullReferenceException.
// See issue #3817.
var fontManager = IntPtr.Zero;
var fontStyle = IntPtr.Zero;
try {
fontManager = SkiaApi.sk_fontmgr_create_default ();
fontStyle = SkiaApi.sk_fontstyle_new (
(int)SKFontStyleWeight.Normal, (int)SKFontStyleWidth.Normal, SKFontStyleSlant.Upright);

// Use legacyMakeTypeface(null) to get the platform default — this uses
// fDefaultStyleSet on Android (which searches "sans-serif", "Roboto",
// then falls back to style set 0). matchFamilyStyle(null) doesn't work
// on Android/NDK/Custom because onMatchFamily(null) returns null.
var matched = SkiaApi.sk_fontmgr_legacy_create_typeface (
fontManager, IntPtr.Zero, fontStyle);
defaultTypeface = matched == IntPtr.Zero
? empty
: new SKTypefaceStatic (matched);
} finally {
if (fontStyle != IntPtr.Zero)
SkiaApi.sk_fontstyle_delete (fontStyle);
if (fontManager != IntPtr.Zero)
SkiaApi.sk_fontmgr_unref (fontManager);
}
}

internal static void EnsureStaticInstanceAreInitialized ()
Expand Down Expand Up @@ -454,6 +475,7 @@ private sealed class SKTypefaceStatic : SKTypeface
internal SKTypefaceStatic (IntPtr x)
: base (x, false)
{
PreventPublicDisposal ();
}

protected override void Dispose (bool disposing) { }
Expand Down
1 change: 1 addition & 0 deletions scripts/infra/tests/tests-netcore.cake
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Task ("Default")
var tfm = "net10.0";
var testAssemblies = new List<string> {
"SkiaSharp.Tests.Console",
"SkiaSharp.Tests.SingletonInit.Console",
"SkiaSharp.Vulkan.Tests.Console",
"SkiaSharp.Direct3D.Tests.Console",
};
Expand Down
1 change: 1 addition & 0 deletions scripts/infra/tests/tests-netfx.cake
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Task ("Default")
var tfm = "net48";
var testAssemblies = new List<string> {
"SkiaSharp.Tests.Console",
"SkiaSharp.Tests.SingletonInit.Console",
"SkiaSharp.Vulkan.Tests.Console",
"SkiaSharp.Direct3D.Tests.Console",
};
Expand Down
14 changes: 14 additions & 0 deletions tests/SkiaSharp.Tests.Console.sln
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SkiaSharp.HarfBuzz", "..\so
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SkiaSharp.Tests.Console", "SkiaSharp.Tests.Console\SkiaSharp.Tests.Console.csproj", "{8E5284C3-5AAF-4902-B12F-84E9172F20E0}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SkiaSharp.Tests.SingletonInit.Console", "SkiaSharp.Tests.SingletonInit.Console\SkiaSharp.Tests.SingletonInit.Console.csproj", "{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SkiaSharp.Vulkan.Tests.Console", "SkiaSharp.Vulkan.Tests.Console\SkiaSharp.Vulkan.Tests.Console.csproj", "{9C3F6513-7B66-4DF3-ADBA-1030FEE9C111}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SkiaSharp.Vulkan.SharpVk", "..\source\SkiaSharp.Vulkan\SkiaSharp.Vulkan.SharpVk\SkiaSharp.Vulkan.SharpVk.csproj", "{A53DD2E5-55C4-4F7C-9316-D7FAD9A6F19F}"
Expand Down Expand Up @@ -85,6 +87,18 @@ Global
{8E5284C3-5AAF-4902-B12F-84E9172F20E0}.Release|x64.Build.0 = Release|x64
{8E5284C3-5AAF-4902-B12F-84E9172F20E0}.Release|x86.ActiveCfg = Release|x86
{8E5284C3-5AAF-4902-B12F-84E9172F20E0}.Release|x86.Build.0 = Release|x86
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Debug|Any CPU.Build.0 = Debug|Any CPU
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Debug|x64.ActiveCfg = Debug|x64
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Debug|x64.Build.0 = Debug|x64
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Debug|x86.ActiveCfg = Debug|x86
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Debug|x86.Build.0 = Debug|x86
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Release|Any CPU.ActiveCfg = Release|Any CPU
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Release|Any CPU.Build.0 = Release|Any CPU
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Release|x64.ActiveCfg = Release|x64
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Release|x64.Build.0 = Release|x64
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Release|x86.ActiveCfg = Release|x86
{B7E2F4A1-3C5D-4E6F-9A8B-1C2D3E4F5A6B}.Release|x86.Build.0 = Release|x86
{9C3F6513-7B66-4DF3-ADBA-1030FEE9C111}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{9C3F6513-7B66-4DF3-ADBA-1030FEE9C111}.Debug|Any CPU.Build.0 = Debug|Any CPU
{9C3F6513-7B66-4DF3-ADBA-1030FEE9C111}.Debug|x64.ActiveCfg = Debug|x64
Expand Down
33 changes: 33 additions & 0 deletions tests/SkiaSharp.Tests.SingletonInit.Console/SKSingletonInitTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using System;
using SkiaSharp;
using Xunit;

namespace SkiaSharp.Tests.SingletonInit
{
// Regression guard for https://github.com/mono/SkiaSharp/issues/3817.
//
// Touching SKFontManager.Default as the very first SkiaSharp type in a fresh
// process used to throw a TypeInitializationException: the SKObject static
// constructor eagerly initializes a cascade of singletons, and the SKTypeface
// initializer in that cascade read the managed SKFontManager.Default and
// SKFontStyle.Normal singletons. When SKFontManager was the first type touched,
// that re-entered the still-running SKFontManager/SKFontStyle initializer on the
// same thread and observed a not-yet-assigned (null) singleton, producing a
// NullReferenceException surfaced as a TypeInitializationException.
//
// This assembly intentionally contains a single test so that this is genuinely
// the first SkiaSharp access in the process.
public class SKSingletonInitTest
{
[Fact]
public void TouchingFontManagerDefaultFirstDoesNotThrow ()
{
var fontManager = SKFontManager.Default;

Assert.NotNull (fontManager);
Assert.NotEqual (IntPtr.Zero, fontManager.Handle);
Assert.NotEqual (IntPtr.Zero, SKFontStyle.Normal.Handle);
Assert.NotEqual (IntPtr.Zero, SKTypeface.Default.Handle);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<Project Sdk="Microsoft.NET.Sdk">

<!--
Issue #3817 regression guard. This is a dedicated test assembly that contains a
single test which touches SKFontManager.Default as the FIRST SkiaSharp type in
the process. Because static constructors run once per process and "dotnet test"
launches a separate test host per assembly, this naturally reproduces the
"first type touched" scenario that used to crash the SKObject static-constructor
eager-initialization cascade. Keeping it in its own assembly is what guarantees
the ordering — do not add other tests or types that touch SkiaSharp here.
-->

<PropertyGroup>
<TargetFrameworks>$(TFMCurrent)</TargetFrameworks>
<TargetFrameworks Condition="$(IsWindows)">$(TargetFrameworks);net48</TargetFrameworks>
<RootNamespace>SkiaSharp.Tests.SingletonInit</RootNamespace>
<AssemblyName>SkiaSharp.Tests.SingletonInit</AssemblyName>
<SignAssembly>false</SignAssembly>
<SkipGenerateAssemblyVersionInfo>true</SkipGenerateAssemblyVersionInfo>
<SkipMDocGenerateDocs>true</SkipMDocGenerateDocs>
<Platforms>AnyCPU;x64;x86</Platforms>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.2" />
<PackageReference Include="xunit" Version="2.9.3" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
<PackageReference Include="XunitXml.TestLogger" Version="3.0.78" />
<PackageReference Include="coverlet.msbuild" Version="10.0.0" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\binding\SkiaSharp\SkiaSharp.csproj" />
</ItemGroup>

<Import Project="..\..\binding\IncludeNativeAssets.SkiaSharp.targets" />

</Project>
32 changes: 32 additions & 0 deletions tests/Tests/SkiaSharp/HandleDictionaryThreadingCollection.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using Xunit;

namespace SkiaSharp.Tests
{
// The dedicated white-box concurrency/lifecycle tests for HandleDictionary/SKObject drive their own
// deterministic interleavings (via gates and barriers on dedicated OS threads) and then join those
// threads against tight deadlines. A failed join is the signal for a genuine product deadlock.
//
// Under xUnit's default parallelism EVERY test class is its own collection and runs concurrently, and
// EVERY SKObject create/dispose/lookup across the whole suite contends on the single global
// HandleDictionary lock. On Windows that lock is a Win32 CRITICAL_SECTION (the #1383 STA-pump fix) with
// no reader concurrency, so the entire parallel suite serializes through one mutex. On a small/slow CI
// agent this can starve a parked dispose thread of CPU/lock ownership for >10s and trip these tight
// deadlines — a false "deadlock" that has nothing to do with the code under test.
//
// Placing all of these classes in one DisableParallelization collection makes them run sequentially in
// xUnit's non-parallel phase, AFTER the parallel phase has drained, so nothing else is hammering the
// global lock while their deadlines are ticking. This does NOT weaken coverage: each test produces its
// own interleaving, and the rest of the suite still exercises the global lock in parallel during the
// parallel phase.
//
// SKManagedStreamConcurrencyTest joins this collection for the same reason: its gated "public Dispose
// while a native lazy read is in-flight" tests park a real OS thread inside a managed Stream.Read() and
// then race a public Dispose against codec teardown under a 30s join deadline. Running it in the
// drained non-parallel phase removes both the global-lock contention above AND any thread-pool
// starvation, so the parked reader and the racing disposer always get CPU before the deadline.
[CollectionDefinition (HandleDictionaryThreadingCollection.Name, DisableParallelization = true)]
public sealed class HandleDictionaryThreadingCollection
{
public const string Name = "HandleDictionary threading (serialized)";
}
}
36 changes: 35 additions & 1 deletion tests/Tests/SkiaSharp/SKCodecTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,11 @@ public unsafe void InvalidStreamIsDisposedImmediately()

Assert.Null(SKCodec.Create(stream));

// Failed Create has no new native owner, so RevokeOwnership(null) runs
// DisposeInternal(): the wrapper is genuinely disposed and deregistered,
// not merely public-dispose-guarded. Hence IsDisposed (not IgnorePublicDispose).
Assert.False(stream.OwnsHandle);
Assert.True(stream.IgnorePublicDispose);
Assert.True(stream.IsDisposed);
Assert.False(SKObject.GetInstance<SKStream>(handle, out _));
}

Expand Down Expand Up @@ -492,5 +495,36 @@ public void CanDecodeData(string image)
Assert.Equal(SKCodecResult.Success, codec.GetPixels(out var pixels));
Assert.NotEmpty(pixels);
}

[SkippableFact]
public void CanDecodeManagedStreamAfterCreate()
{
// Regression: SKCodec.Create(Stream) wraps the managed .NET stream and the
// codec decodes lazily. The underlying managed stream MUST stay readable
// until the codec is disposed — the singleton/lifecycle rework must not let
// the reparented wrapper's ownership transfer eagerly close the .NET stream.
using var stream = new MemoryStream(File.ReadAllBytes(Path.Combine(PathToImages, "baboon.png")));
using var codec = SKCodec.Create(stream);
Assert.NotNull(codec);

Assert.Equal(SKCodecResult.Success, codec.GetPixels(out var pixels));
Assert.NotEmpty(pixels);
}

[SkippableFact]
public void CanDecodeNonSeekableManagedStreamAfterCreate()
{
// Non-seekable managed streams route through SKFrontBufferedManagedStream
// (a nested SKManagedStream). The lazy decode must still reach the underlying
// .NET stream through the front buffer + nested wrapper without it being
// closed early by the ownership transfer.
using var backing = new MemoryStream(File.ReadAllBytes(Path.Combine(PathToImages, "baboon.png")));
using var nonSeekable = new NonSeekableReadOnlyStream(backing);
using var codec = SKCodec.Create(nonSeekable);
Assert.NotNull(codec);

Assert.Equal(SKCodecResult.Success, codec.GetPixels(out var pixels));
Assert.NotEmpty(pixels);
}
}
}
8 changes: 6 additions & 2 deletions tests/Tests/SkiaSharp/SKColorFilterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ public class SKColorFilterTest : SKTest
[Trait(Traits.Category.Key, Traits.Category.Values.Smoke)]
public void StaticSrgbToLinearIsReturnedAsTheStaticInstance()
{
var expected = SKColorFilter.CreateSrgbToLinearGamma();
var handle = SkiaApi.sk_colorfilter_new_srgb_to_linear_gamma();
try
{
var cs = SKColorFilter.GetObject(handle);
Assert.Equal("SKColorFilterStatic", cs.GetType().Name);
Assert.Same(expected, cs);
Assert.True(cs.IgnorePublicDispose);
}
finally
{
Expand All @@ -26,11 +28,13 @@ public void StaticSrgbToLinearIsReturnedAsTheStaticInstance()
[SkippableFact]
public void StaticLinearToSrgbIsReturnedAsTheStaticInstance()
{
var expected = SKColorFilter.CreateLinearToSrgbGamma();
var handle = SkiaApi.sk_colorfilter_new_linear_to_srgb_gamma();
try
{
var cs = SKColorFilter.GetObject(handle);
Assert.Equal("SKColorFilterStatic", cs.GetType().Name);
Assert.Same(expected, cs);
Assert.True(cs.IgnorePublicDispose);
}
finally
{
Expand Down
Loading
Loading