From 5fc4f0b1dc3efc10667bc6fc99b0e9132fa60b8a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:55:44 +0000 Subject: [PATCH 1/3] Fix double-free in HarfBuzzSharp.Buffer.UnicodeFunctions getter hb_buffer_get_unicode_funcs returns a borrowed (transfer-none) reference owned by the buffer. The getter wrapped it in an owning UnicodeFunctions, so disposing the returned wrapper called hb_unicode_funcs_destroy and over-freed a handle still referenced by the live buffer (double-free / premature destroy). Make UnicodeFunctions track ownership and wrap the borrowed getter result non-owning so Dispose is a no-op for it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- binding/HarfBuzzSharp/Buffer.cs | 4 +++- binding/HarfBuzzSharp/UnicodeFunctions.cs | 12 +++++++++- .../Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs | 23 +++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/binding/HarfBuzzSharp/Buffer.cs b/binding/HarfBuzzSharp/Buffer.cs index 4241cdf84ee..afb4047a01d 100644 --- a/binding/HarfBuzzSharp/Buffer.cs +++ b/binding/HarfBuzzSharp/Buffer.cs @@ -132,7 +132,9 @@ public int Length { public UnicodeFunctions UnicodeFunctions { get { - var r = new UnicodeFunctions (HarfBuzzApi.hb_buffer_get_unicode_funcs (Handle)); + // hb_buffer_get_unicode_funcs returns a borrowed (transfer-none) reference + // owned by the buffer, so the wrapper must not destroy it on dispose. + var r = new UnicodeFunctions (HarfBuzzApi.hb_buffer_get_unicode_funcs (Handle), owns: false); GC.KeepAlive (this); return r; } diff --git a/binding/HarfBuzzSharp/UnicodeFunctions.cs b/binding/HarfBuzzSharp/UnicodeFunctions.cs index 7228a5be4c3..6e6e9044010 100644 --- a/binding/HarfBuzzSharp/UnicodeFunctions.cs +++ b/binding/HarfBuzzSharp/UnicodeFunctions.cs @@ -16,11 +16,21 @@ public unsafe class UnicodeFunctions : NativeObject public static UnicodeFunctions Empty => emptyFunctions.Value; + // true when this wrapper owns the native handle and must destroy it on dispose; + // false when it merely borrows a handle owned by another object (e.g. a buffer). + private readonly bool owns = true; + internal UnicodeFunctions (IntPtr handle) : base (handle) { } + internal UnicodeFunctions (IntPtr handle, bool owns) + : base (handle) + { + this.owns = owns; + } + public UnicodeFunctions (UnicodeFunctions parent) : base (IntPtr.Zero) { if (parent == null) @@ -196,7 +206,7 @@ protected override void Dispose (bool disposing) => protected override void DisposeHandler () { - if (Handle != IntPtr.Zero) { + if (owns && Handle != IntPtr.Zero) { HarfBuzzApi.hb_unicode_funcs_destroy (Handle); } } diff --git a/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs b/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs index f21426f0b2b..56babce49e0 100644 --- a/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs +++ b/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs @@ -145,6 +145,29 @@ public void ShouldSetDecomposeDelegate() Assert.Equal(1337, first); Assert.Equal(7331, second); } + [Fact] + public void BufferUnicodeFunctionsGetterDoesNotOwnBorrowedHandle() + { + // hb_buffer_get_unicode_funcs returns a borrowed (transfer-none) reference + // owned by the buffer. Disposing the wrapper returned by the getter must not + // destroy that shared handle out from under the still-alive buffer. + var destroyed = false; + + var unicodeFunctions = new UnicodeFunctions(UnicodeFunctions.Default); + unicodeFunctions.SetScriptDelegate((f, cp) => Script.Unknown, () => destroyed = true); + + // The buffer is intentionally left alive (and its funcs referenced) for the + // whole test, so the funcs must remain valid. + var buffer = new Buffer(); + buffer.UnicodeFunctions = unicodeFunctions; + + var borrowed = buffer.UnicodeFunctions; + borrowed.Dispose(); + unicodeFunctions.Dispose(); + + Assert.False(destroyed, "buffer.UnicodeFunctions getter over-freed a borrowed handle."); + + GC.KeepAlive(buffer); } } } From 31fcb310726caf50245584b1f5bcbc0056c5d267 Mon Sep 17 00:00:00 2001 From: Copilot App <223556219+Copilot@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:21:26 +0200 Subject: [PATCH 2/3] Fix malformed test braces breaking all Tests legs The regression test BufferUnicodeFunctionsGetterDoesNotOwnBorrowedHandle was inserted inside ShouldSetDecomposeDelegate's body: the method's closing brace was missing and the new [Fact] method was nested as an invalid public local function. This produced two compile errors in SkiaSharp.Tests.csproj: HBUnicodeFuncsTest.cs(149,3): error CS0106: The modifier 'public' is not valid for this item HBUnicodeFuncsTest.cs(173,2): error CS1513: } expected A non-compiling test assembly caused every Tests leg to fail on every platform (WASM/Windows/Linux/macOS/Catalyst/Android/iOS) while the Build Managed jobs passed. Close ShouldSetDecomposeDelegate properly and de-nest the new test as a class member. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 278bdb81-233e-4f16-bbf3-ec801600bd12 --- tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs b/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs index 56babce49e0..c1959d7e53a 100644 --- a/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs +++ b/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs @@ -145,6 +145,8 @@ public void ShouldSetDecomposeDelegate() Assert.Equal(1337, first); Assert.Equal(7331, second); } + } + [Fact] public void BufferUnicodeFunctionsGetterDoesNotOwnBorrowedHandle() { From efba6ff8dd822c1b6cda53ad554e4538f1210851 Mon Sep 17 00:00:00 2001 From: Copilot App <223556219+Copilot@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:25:51 +0200 Subject: [PATCH 3/3] Dispose the Buffer in the regression test Addresses Copilot review feedback: use 'using var buffer' so the buffer stays alive through the assertion and is disposed cleanly at scope exit, matching the disposal convention of the other HarfBuzzSharp tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 278bdb81-233e-4f16-bbf3-ec801600bd12 --- tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs b/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs index c1959d7e53a..4fed4074c78 100644 --- a/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs +++ b/tests/Tests/HarfBuzzSharp/HBUnicodeFuncsTest.cs @@ -158,9 +158,9 @@ public void BufferUnicodeFunctionsGetterDoesNotOwnBorrowedHandle() var unicodeFunctions = new UnicodeFunctions(UnicodeFunctions.Default); unicodeFunctions.SetScriptDelegate((f, cp) => Script.Unknown, () => destroyed = true); - // The buffer is intentionally left alive (and its funcs referenced) for the - // whole test, so the funcs must remain valid. - var buffer = new Buffer(); + // The buffer is kept alive (via using) so its funcs reference stays valid + // through the assertion, and is disposed cleanly at the end of the scope. + using var buffer = new Buffer(); buffer.UnicodeFunctions = unicodeFunctions; var borrowed = buffer.UnicodeFunctions; @@ -168,8 +168,6 @@ public void BufferUnicodeFunctionsGetterDoesNotOwnBorrowedHandle() unicodeFunctions.Dispose(); Assert.False(destroyed, "buffer.UnicodeFunctions getter over-freed a borrowed handle."); - - GC.KeepAlive(buffer); } } }