Windows only - Random crash, for version 2.2 #674
Comments
|
For |
|
@cameronwhite, the first version that I used with for 2.2, with narrow toolbar for tools and scroll bar, didn't crash. |
|
Thanks, yeah there isn't any additional info in the About window unfortunately I think the main one to just compare against is the regular |
|
Those things are hard to debug as you are on the mercy of the gc. For example in debug mode or with a debugger attached the gc will behave differently. Perhaps your machine is more beefy and the gc does not need to collect objects as frequently. You can try to force collection but from my experience this is not guaranteed to result in collection of the wanted object. You can try to call: GC.Collect();
GC.WaitForPendingFinalizers();To force collection. |
|
test 1: master, latest build 7470015712 (narrow toolbar with scrollbar) |
|
In 13ebeda I added a |
|
From what I saw, the "-debug" was added in master, but I've encountered the problem in build from branch "toolbox-layout". |
|
Sure, I've rebased it on the latest master branch: https://github.com/PintaProject/Pinta/actions/runs/7523936006 |
|
still getting crash in exe from 7523936006 |
|
Perhaps an idea is to have some print to the console somewhere in the stacktrace and print the name of the class which creates the problem? |
|
@badcel , as you wrote it maybe issue with GC, so I don't think it will help. |
|
The collection happens asynchronous, so there is no way back to the creating code. But if there is a way to detect the type of the handle which gets freed it is perhaps possible to narrow down the classes which could cause the problem. |
Yeah I think that would be really helpful for tracking down issues 👍 |
|
I'm not familiar with GC to that level. Most examples from MS required usage of windbg. |
|
@iksi4prs see: gircore/gir.core#1004 Pinta uses GirCore which is compiled with source link support. So you can actually take a look into every file from pintas source. |
There also shouldn't be a GtkSharp.dll since that's from the GTK3 version of Pinta (we use gir.core now) - maybe you have some leftover files from an old installation? |
I just run the installer every time. Looks like it didn't clean previous installation during upgrade. |
|
I've installed dev env.
I see that's GObject is external to Pinta, |
|
A lot of the subclasses of You could also see if you can inspect |
|
@cameronwhite , @badcel . I saw pr in gircore/gir.core#1004 When it is expected to available in Pinta's build ? |
|
It makes sense that object is null. The handle gets released because there are no instances anymore in the first place, which requires the instance to be null. The problem is that @cameronwhite raised the concern that, reading data from a pointer if an I would not like to merge this without confirmation that it does not result in a new exception. This would either require you to build GirCore yourself with the fix and afterwards build Pinta using your own local GirCore build to confirm it works or I find another solution. But this is not yet very obvious to me. There are lots of possible ways around this, but I want the code to be as clean and simple as possible. |
|
I'm trying to aim for a more global solution, in case occurs again in future. What about adding logs in ObjectHandle.cs ? eg, some "pseudo" code, with names that may not be accurate: |
|
@iksi4prs there are already debug logs in place in I think it is fine to compile GirCore locally in debug mode if a project has such kind of problems which requires actual debug output. (The log in |
|
@badcel ,maybe in this case (since exception cant be caught) it will be nice in the future to add "stopwatch" style logger, using disposable. Where ctor of Logger logs the "Enter" and "Dispose" logs the "Exit", then easier to find places where no matching "Exit" and "Enter" pairs. |
|
I've built Pinta with Gir.Core and added some logs. Where for other case, this is also the stack of the mapping itself: My familiarity with the code is 0. |
|
@iksi4prs my advice would be to do the following steps:
Or did you already do this and the result is that it's |
|
@iksi4prs If it never gets collected I think this is a hint that it is not a double free bug because there is one obsolete ref which keeps the instance alive forever. Can you check if the |
|
Looking in the debugger, it does seem to be a WeakReference instance. This was my sample code. using System;
var application = Adw.Application.New("org.gir.core", Gio.ApplicationFlags.FlagsNone);
application.OnActivate += (sender, args) =>
{
var label = Gtk.Label.New("Hello world");
label.HasTooltip = true;
label.OnQueryTooltip += (_, args) =>
{
args.Tooltip.SetText("This is a tooltip");
return true;
};
var window = Gtk.ApplicationWindow.New((Adw.Application) sender);
window.Title = "Window";
window.SetDefaultSize(300, 300);
window.SetChild(label);
window.Show();
};
// Run the GC more frequently
GLib.Functions.TimeoutAdd(0, 100, () =>
{
GC.Collect();
GC.WaitForPendingFinalizers();
return true;
});
return application.RunWithSynchronizationContext(null); |
|
I also notice a warning Since this happens after moving the cursor away to close the tooltip, I wonder if there's a timing issue where if the tooltip is destroyed sometime later on (whenever the GC collects the C# object) instead of right away, it has a reference to something that is now stale? |
|
Immediately cleaning up the C# objects does seem to work .. I haven't hit a crash so far with the following changes: I changed the callback in the sample code to var tooltip = args.Tooltip;
tooltip.SetText("This is a tooltip");
tooltip.Dispose();And then also changed finally
{
foreach (var val in paramValues)
val.Dispose();
} |
|
@cameronwhite did you try to apply only the change in I wonder if it is correct to dispose the Tolltip in the sample app before the call returns? Looks a little bit odd for me. In the Closure implementation there is a copy of the But I'm not sure if the I will check. Perhaps this is the bug we are looking for. |
|
Yeah, both of those changes seemed necessary to fix all the crashes, although the change in the sample app is definitely a hack and isn't a good fix.
|
|
Hm, I wrote this test: private class Foo : GObject.Object
{
public Foo() : base(true, Array.Empty<ConstructArgument>())
{
}
}
[TestMethod]
public void ValueReferenceCountTest()
{
var foo = new Foo();
Marshal.PtrToStructure<GObject.Internal.ObjectData>(foo.Handle).RefCount.Should().Be(1);
var v1 = new Value(foo);
Marshal.PtrToStructure<GObject.Internal.ObjectData>(foo.Handle).RefCount.Should().Be(2);
var ownedHandle = new GObject.Internal.ValueUnownedHandle(v1.Handle.DangerousGetHandle()).OwnedCopy();
Marshal.PtrToStructure<GObject.Internal.ObjectData>(foo.Handle).RefCount.Should().Be(3);
var v2 = new GObject.Value(ownedHandle);
Marshal.PtrToStructure<GObject.Internal.ObjectData>(foo.Handle).RefCount.Should().Be(3);
}Unfortunately the test is successfull under linux. The code used here is the code used if the value array is copied. So from my point of view everything looks correct here. @cameronwhite i executed your sample code under linux and the app crashed after the tooltip gets hidden and a warning is emited. It seems to be the same error like on windows. Can you confirm? I think the next step is that I convert the sample code to C just to be sure that the Gdk-Warning is not responsible for the crash. |
|
On macOS I get a different but probably equivalent warning: And then the sample application hangs afterwards. Code: #include <adwaita.h>
static gboolean
on_query_tooltip(GtkWidget *widget,
int x,
int y,
gboolean keyboard_mode,
GtkTooltip *tooltip,
gpointer data)
{
gtk_tooltip_set_text(tooltip, "This is a tooltip");
return TRUE;
}
static void
activate_cb(GtkApplication *app)
{
GtkWindow *window = GTK_WINDOW(gtk_application_window_new(app));
GtkWidget *label = gtk_label_new("Hello World");
gtk_widget_set_has_tooltip(label, TRUE);
g_signal_connect(label, "query-tooltip", G_CALLBACK(on_query_tooltip), NULL);
gtk_window_set_title(window, "Window");
gtk_window_set_default_size(window, 300, 300);
gtk_window_set_child(window, label);
gtk_window_present(window);
}
int main(int argc,
char *argv[])
{
g_autoptr(AdwApplication) app = NULL;
app = adw_application_new("org.gir.core", G_APPLICATION_DEFAULT_FLAGS);
g_signal_connect(app, "activate", G_CALLBACK(activate_cb), NULL);
return g_application_run(G_APPLICATION(app), argc, argv);
}Meson build script |
|
The test works fine for me on Windows and doesn't crash. I didn't see any console output either Thinking about this more, one other possible issue is that the final My previous workaround, which immediately called Dispose(), also would have had the side effect of making the |
|
@cameronwhite this is a good idea. Perhaps disposing the Tooltip directly is no problem because GTK itself holds another reference and this would avoid the GC to kick in from another thread. I will check if this is true and try some hacks to verify your assumption. (For me it sounds very plausible.) |
|
I made a quick test by replacing the ToggleRef.Dispose() method with: public void Dispose()
{
GLib.Functions.TimeoutAdd(GLib.Constants.PRIORITY_DEFAULT, 50, () =>
{
Internal.Object.RemoveToggleRef(_handle, _callback, IntPtr.Zero);
return false;
});
}and that seemed to work in the testing I did on macOS and Windows |
|
I tested your code, too on fedora and it worked. I have an alternate implementation, which seems to work, too. What do you think? public void Dispose()
{
var sourceFunc = new SourceFuncAsyncHandler(() =>
{
Internal.Object.RemoveToggleRef(_handle, _callback, IntPtr.Zero);
return false;
});
MainContext.Invoke(MainContextUnownedHandle.NullHandle, sourceFunc.NativeCallback, IntPtr.Zero);
}It avoids a timeout and invokes the Docs: MainContext.Invoke / TimeOutAdd In general I wonder if it is correct to call the |
|
I made another test, which involves only GObject: using GObject;
public class Program
{
public static async Task Main(string[] args)
{
GObject.Module.Initialize();
Console.WriteLine("Hello, World!");
var p = new Program();
p.Do();
await Task.Delay(5000);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
await Task.Delay(5000);
}
public void Do()
{
var foo = new Foo();
}
}
public class Foo : GObject.Object
{
public Foo() : base(true, Array.Empty<ConstructArgument>())
{
}
}I don't know what must be altered so it works with the solution using But the original So I would tend to say that the call of |
|
If I add some extra print statements like this public void Dispose()
{
System.Console.WriteLine("disposing");
var sourceFunc = new GLib.Internal.SourceFuncAsyncHandler(() =>
{
System.Console.WriteLine("remove toggle ref");
Internal.Object.RemoveToggleRef(_handle, _callback, IntPtr.Zero);
return false;
});
MainContext.Invoke(MainContextUnownedHandle.NullHandle, sourceFunc.NativeCallback, IntPtr.Zero);
System.Console.WriteLine("done disposing");
}Then in the GObject test it looks like the callback is immediately executed. So it must be meeting the conditions mentioned in https://docs.gtk.org/glib/method.MainContext.invoke.html to execute right away, I think because the main context hasn't been acquired by another thread So I think MainContext.Invoke() sounds fine to me - my only thought with the timeout was that it would allow batching together the RemoveToggleRef calls if there are a bunch of objects being disposed, if there is any overhead from creating and scheduling the callback to run on the main thread for every object. But that can probably be done later if we find it actually necessary |
|
The fix just got merged in gir.core. With the next release it should be fixed. |
|
@badcel and @cameronwhite , thanks for time and effort putting in fixing this one. @cameronwhite , this issue was arisen after changes for "toolbox-layout" Do you have plans to merge it master, or need any more feedback, or else ? (see original discussion: #654 ) |
|
Excellent, thanks for merging the fix! I think the |
|
Are you planning on testing it with the gir.core fix? This would actually be great, to see potential performance problems. |
|
I'll try rebuilding locally with the new gir.core fix and do some testing. |
|
I've tested on Windows with Pinta built against the latest gir.core and verified that the tooltip crash seems to be fixed (along with the similar crash that occurred in #766) I haven't noticed any performance issues, but other than a few cases like the tooltip signal handler Pinta typically isn't rapidly creating GObject instances (the hot code paths mostly involve Cairo) |
- This contains a fix for the random crashes encountered on Windows - The Cairo.Matrix class now has a more convenient constructor - Gdk.FileList.GetFiles() is not being generated and requires a manual binding for now, but GLib.SList methods are now generated Fixes: #674
- This contains a fix for the random crashes encountered on Windows - The Cairo.Matrix class now has a more convenient constructor - Gdk.FileList.GetFiles() is not being generated and requires a manual binding for now, but GLib.SList methods are now generated Fixes: #674
- This contains a fix for the random crashes encountered on Windows - The Cairo.Matrix class now has a more convenient constructor - Gdk.FileList.GetFiles() is not being generated and requires a manual binding for now, but GLib.SList methods are now generated Fixes: #674
- This contains a fix for the random crashes encountered on Windows - The Cairo.Matrix class now has a more convenient constructor - Gdk.FileList.GetFiles() is not being generated and requires a manual binding for now, but GLib.SList methods are now generated Fixes: #674
Description
App crashes.
See more information about optional cause in discussion/comment:
#654 (reply in thread)
To Reproduce
Random.
I've noticed that after first install, it works fine for few seconds.
But trying to run app again fails, namely crash is even before first window is created (or at least before it becomes visible)
Additional Info
Logs from "Windows's events viewer"
Application: Pinta.exe CoreCLR Version: 8.0.23.53103 .NET Version: 8.0.0 Description: The process was terminated due to an unhandled exception. Exception Info: System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt. Stack: at GObject.Internal.Object.RemoveToggleRef(IntPtr, GObject.Internal.ToggleNotify, IntPtr) at GObject.Internal.Object.RemoveToggleRef(IntPtr, GObject.Internal.ToggleNotify, IntPtr) at GObject.Internal.ObjectMapper.Unmap(IntPtr) at GObject.Internal.ObjectHandle.ReleaseHandle() at System.Runtime.InteropServices.SafeHandle.InternalRelease(Boolean) at System.Runtime.InteropServices.SafeHandle.Finalize()Faulting application name: Pinta.exe, version: 2.2.0.0, time stamp: 0x65410000 Faulting module name: coreclr.dll, version: 8.0.23.53103, time stamp: 0x65411660 Exception code: 0xc0000005 Fault offset: 0x00000000001e337a Faulting process id: 0x0xBC80 Faulting application start time: 0x0x1DA3F1C8D463DE9 Faulting application path: C:\Program Files\Pinta\bin\Pinta.exe Faulting module path: C:\Program Files\Pinta\bin\coreclr.dll Report Id: fa0399c5-aff9-4e9b-b43c-0b1429429ed9 Faulting package full name: Faulting package-relative application ID:Faulting application name: Pinta.exe, version: 2.1.1.0, time stamp: 0x63c9df9f Faulting module name: KERNELBASE.dll, version: 10.0.22621.2792, time stamp: 0x3091b6fb Exception code: 0x80000003 Fault offset: 0x000000000010cd82 Faulting process id: 0x0x6A54 Faulting application start time: 0x0x1DA3F1CC7555BDF Faulting application path: C:\Program Files\Pinta\Pinta.exe Faulting module path: C:\WINDOWS\System32\KERNELBASE.dll Report Id: 8d3cb891-26e0-42d4-943e-630d861f029b Faulting package full name: Faulting package-relative application ID:Version
Operating System:
Windows 11 Pro
Version 22H2
OS build 22621.2861
Pinta:
Pinta 2.2, from this branch: (not all dev branch have this problem)
https://github.com/PintaProject/Pinta/actions/runs/7405489857
The text was updated successfully, but these errors were encountered: