[BUG] SKNativeObject
instances are not kept alive during native API calls
#2821
Labels
SKNativeObject
instances are not kept alive during native API calls
#2821
Description
Hi there, I'm currently integrating SkiaSharp in a project, and it seems to work well so far. However, when looking through the source code, unless I'm missing something, I think there's an issue/bug in how the native handle of
SKNativeObject
(e.g.SKPaint
) is freed by the finalizer (i.e. when you don't explicitly callDispose()
).Generally, when a managed object wraps a native handle/pointer, you want to ensure that the handle is not released "prematurely", i.e. when it is still in use. See the following comment in
GC.KeepAlive()
:For example, consider the following user code, where a
SKPaint
is created without ausing
statement (so it isn't disposed explicitly):canvas.DrawLine(...)
will retrieve theHandle
of theSKPaint
andSKCanvas
, and pass it to the nativesk_canvas_draw_line
function:SkiaSharp/binding/SkiaSharp/SKCanvas.cs
Lines 89 to 94 in a290ccf
However, because the
SKPaint
object in the example above isn't used after thecanvas.DrawLine()
call, it may become eligible for garbage collection right after retrieving itsHandle
property inSKCanvas.DrawLine()
.This could mean that the GC finalizer already calls
SkiaApi.sk_compatpaint_delete
(from the finalizer thread) whileSkiaApi.sk_canvas_draw_line()
is still executing, or even beforeSkiaApi.sk_canvas_draw_line
is called, which would cause undefned behavior, like invalid memory access (e.g. it could cause Access Violations such as in #2794, though I don't know whether that issue would be caused by this one).For example, this might be an issue when using
RichString
fromTopten.RichTextKit
, which doesn't seem to explicitly dispose SK objects likeSKFont
etc., and relies on the finalizer freeing the handle.A solution is to add
GC.KeepAlive()
calls after a native API call (as described in that method's comment), to keep the objects alive from which the handles are retrieved. For example, withSKCanvas.DrawLine()
, the code could be changed to this:The
GC.KeepAlive()
calls here ensure that the objects (SKPaint
andSKCanvas
) don't become eligible for GC (and thus their finalizer which would delete the native handles won't be called) until after theSkiaApi.sk_canvas_draw_line
call returns.This is also e.g. how
dotnet/winforms
handles it: https://github.com/dotnet/winforms/blob/897c9d87e973be3724375f8b272e82a5c6692070/src/System.Drawing.Common/src/System/Drawing/Bitmap.cs#L179-L180What do you think?
Thanks!
Code
(see above)
Expected Behavior
SKNativeObject
instances should be kept alive during native API calls where theirHandle
is passed, until after the native API call returns.Actual Behavior
SKNativeObject
instances are not kept alive, which could cause the GC calling the finalizer which would delete the handle via functions such asSkiaApi.sk_compatpaint_delete
while or before another thread still uses these handles in a native API call likeSkiaApi.sk_canvas_draw_line
, causing undefined behavior.Version of SkiaSharp
3.x (Alpha)
Last Known Good Version of SkiaSharp
Other (Please indicate in the description)
IDE / Editor
Visual Studio (Windows)
Platform / Operating System
Windows
Platform / Operating System Version
Windows 10 Pro Version 22H2 (x64)
Devices
No response
Relevant Screenshots
No response
Relevant Log Output
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: