Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

cairo: fix remaining leaks when finalizer gets called #92

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

knocte
Copy link
Contributor

@knocte knocte commented Nov 4, 2013

Apply same finalizer leak fix to the rest of IDisposable-ownership
cairo classes as the one recently committed for Cairo.Context[1].

[1] 41eeecb

Apply same finalizer leak fix to the rest of IDisposable-ownership
cairo classes as the one recently committed for Cairo.Context[1].

[1] mono@41eeecb
Since some finalizer leaks started to get fixed ([1],[2]) hangs, freezes,
and X11 crashes were a bit more frequent. Queuing the refcount decrease
using a GLib.Timeout fixes them.

[1] mono@41eeecb
[2] mono@c28b352
@knocte
Copy link
Contributor Author

knocte commented Nov 8, 2013

@bl8 I pushed to this PR the extra commit with the fixes I told you yesterday, it's very important that this (or a similar approach to respect the rule "Don't try to use Cairo in threads other than the main (Gdk) thread." mentioned in http://www.mono-project.com/Mono.Cairo) gets committed before the 2.99.2 release IMO. I know that maybe cairo-sharp should not depend on glib-sharp because there is no technical reason for it, but native cairo already has an optional dependency on native glib, so I don't think the dependency at the binding level is that bad.

@bl8
Copy link
Contributor

bl8 commented Nov 17, 2013

I built gtk-sharp from the latest git master with your commit c28b352 (cairo: fix remainind leaks) applied on top, and I tried running Banshee git master.
I didn't see any crashes or freezes.
Could you try to reproduce those issues and provide some stacktraces or any info you have ?

Make sure you use the latest gtk-sharp from git master, I've just fixed another leak in the Widget.Drawn signal.

@knocte
Copy link
Contributor Author

knocte commented Nov 18, 2013

@bl8, take in account that with your fixes in e48ac63 and 51f102b the freezes and hangs are much more difficult to reproduce (because in this case it's not the GC calling the destroy() method), so after I reverted those two commits, I could reproduce two problems which I pasted here: https://bugzilla.xamarin.com/show_bug.cgi?id=16321

If I try a bit harder, it's possible that I'll reproduce the 3rd problem I also experienced: a X-Windows crash (BadMatch error, I think).

@knocte
Copy link
Contributor Author

knocte commented Nov 18, 2013

So, just to make it doubly clear: the fact that your commits e48ac63 and 51f102b make this problem much harder to reproduce (or impossible to reproduce with banshee maybe) doesn't mean that the bug doesn't exist, because other people's code (instead of the generated code) may also forget to call Dispose(), which then means the finalizer would act, which means that the sentence "Don't try to use Cairo in threads other than the main (Gdk) thread." (from http://www.mono-project.com/Mono.Cairo) is not respected, which brings all sorts of problems (in the same way if you forget to call Gtk from the proper thread).

@bl8
Copy link
Contributor

bl8 commented Dec 1, 2013

I've pushed the first commit ("fix remaining leaks...") to git master.
Let's see if it causes any issues for people, I haven't seen any on my side.

Base automatically changed from master to main March 9, 2021 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants