Graphics.DrawImage leaks memory on Linux (GTK2 and 3)

[kivarsen]

Calling Graphics.DrawImage() appears to leak memory when using the GTK2 / 3 handlers. I first noticed this when an app that calls Graphics.DrawImage() on every Paint event would bring a Linux system to its knees after just a minute or two.

To reproduce the issue: simply run Eto.Test.Gtk2 under Linux and toggle back and forth between the "Bitmap" and "Bitmap GetPixel" sections a few dozen times while watching memory usage in a tool like top. On my machine, the resident size of the process grew from 35M to 119M after toggling back and forth 30 times, and will continue to grow without bounds if you keep doing it. (For what it's worth, the value reported by the "Memory" button in the test app doesn't necessary show a growing number.)

I also see exceptions in MonoDevelop's Application Output section, with the critical line being:

"Cairo.Pattern is leaking, programmer is missing a call to Dispose"

I think the problem can be found under Eto/Source/Eto.Gtk/Drawing/BitmapHandler.cs, around line 165 in the current develop branch, where the following code can be found:

if (EtoEnvironment.Platform.IsMac)
    pattern.Dispose();

Simply removing the "if" condition seems to fix the problem. I ran a program with a 60fps redraw loop for a few hours and memory usage stayed reasonable.

These lines were introduced in commit 62e47... on Jun 22, 2014 with log message:

Gtk: Fix dispose warnings when running on OS X

Similar additions can be found in IconHandler.cs (lines 67/68) and TextureBrushHandler.cs (lines 39/40).

I can see that TextureBrushHandler.DrawImage() is also leaking memory under Linux by running the Draw Loop demo with Textures & Gradients enabled. I don't know if IconHandler.DrawImage() ever gets called in the demo app, but since it's using a Cairo.SurfacePattern I have to assume that it should get disposed as well.

In any case, removing the check for EtoEnvironment.Platform.IsMac in these three places should fix the problem.

(For what it's worth, removing the call to Dispose() and testing on the Mac revealed the same exceptions and memory leakage behavior, so it doesn't seem that there would be a reason to call Dispose() on one platform and not on another.)

[cwensley]

The Dispose() call was only done on OS X as it actually caused a crash on windows and linux. Are you using mono 4.x by any chance? From my testing a while back, adding the Dispose() will cause the crash on linux if using 3.2.8, which is standard on latest ubuntu. However, it didn't have the same memory leak.

I'm not sure the best approach to make the call to Dispose() when needed, perhaps by checking the gtk-sharp version instead, though we'd have to figure out what version it was changed in. Hopefully they didn't keep the same version otherwise there'd be no way to know if it is needed or not.

[kivarsen]

Hi Curtis,

Thanks for the response -- good to learn the history behind this. You'd sure think it would be safe to call Dispose() on an IDisposable! :-)

I've been doing my testing on Linux Mint 17 (32-bit) with Mono 3.2.8 and GTK# 2.12.10 inside VirtualBox. Any idea what version of GTK# you were running at the time?

I wonder if this is a 32-bit vs 64-bit issue. I'm downloading the 64-bit ISO for Ubuntu 14.04 right now, and tomorrow I'll try to reproduce the crash you were seeing. Will let you know what I find.

Cheers, Kevin

On Thu, Aug 6, 2015 at 9:17 PM, Curtis Wensley notifications@github.com wrote:

The Dispose() call was only done on OS X as it actually caused a crash on windows and linux. Are you using mono 4.x by any chance? From my testing a while back, adding the Dispose() will cause the crash on linux if using 3.2.8, which is standard on latest ubuntu. However, it didn't have the same memory leak.

I'm not sure the best approach to make the call to Dispose() when needed, perhaps by checking the gtk-sharp version instead, though we'd have to figure out what version it was changed in. Hopefully they didn't keep the same version otherwise there'd be no way to know if it is needed or not.

— Reply to this email directly or view it on GitHub https://github.com/picoe/Eto/issues/367#issuecomment-128592717.

[kivarsen]

Hi Curtis,

After a fair amount of testing, I am still unable to cause any crashes by enabling the call to Dispose() in Gtk's BitmapHandler. I've tested under Linux Mint 17 (32-bit) and Ubuntu 14.04 (64-bit), both of which use Mono 3.2.8. I even tried reverting my Eto repo to the June 22, 2014 commit and compiling against that (in case some other GTK interaction code from an older version of Eto had the side effect of causing Dispose() to crash), and things still seem to work perfectly well when Dispose() is called.

Do you recall what distribution and version of Linux you were running when you saw this crash? I can try setting that up to see if I can reproduce it. As you say, it's quite possible that the crash was caused by an underlying bug in an old version of gtk-sharp, but at the moment I'm not quite sure how to easily revert to an old version for testing.

[cwensley]

Hm, perhaps it was mono 2.10.. it has been a while. If adding the Dispose() works on all supported versions of ubuntu and other popular distros (are there any?) then I'd say it should be added in.

[kivarsen]

Aha - testing under Ubuntu 12.04 (Mono 2.10.8) using an older commit that could still be loaded by MonoDevelop 2.8, I was able to reproduce the crash when calling Dispose(). The error is as follows:

mono: /build/buildd/cairo-1.10.2/src/cairo-pattern.c:822: cairo_pattern_destroy: Assertion `((*&(&pattern->ref_count)->ref_count) > 0)' failed. Stacktrace: at (wrapper managed-to-native) Cairo.NativeMethods.cairo_pattern_destroy (intptr) <IL 0x00023, 0xffffffff> at Cairo.Pattern.Destroy () <IL 0x0001b, 0x00037> (truncated)

Digging into it a bit more, it seems like the behavior of Gdk.CairoHelper.SetSourcePixbuf (which appears to be a simple wrapper around the native gdk_cairo_set_source_pixbuf function) has changed at some point. When running in Ubuntu 12.04, there doesn't appear to be a memory leak. However, when running under Ubuntu 14.04, memory usage grows at the moment when SetSourcePixbuf is called and doesn't seem to get released until Dispose() is called.

As another interesting test, if I modify DrawImage to call SetSourcePixbuf (for the same source) multiple times, I see no memory growth in Ubuntu 12.04, but I do see memory grow each time it is called in Ubuntu 14.04.

So at the moment it seems like there might be an issue in the actual GDK (or perhaps Cairo) libraries. If I get time, I'll try to do some lower-level testing of these libraries to see if I can sort out the "right" (generalized) way to make sure this memory is getting managed correctly.

[cwensley]

Thanks again for all your testing, it really helps!

I did some testing as well, and it seems like my other changes to remove use of obsolete api's are also not compatible with ubuntu 12.04. In order to support it, we'd have to revert some of those changes. I am hoping to work towards removing .net 4.0, so it might make most sense to just target mono 3.2.x/ubuntu 14.04 and greater. This means we should be able to call Dispose() without any conditions to fix this issue.

If you (or anyone) would prefer to support mono 2.10 at least until ubuntu 12.04 is EOL, I would be okay with that, however I am leaning towards removing mono 2.10 support.

[kivarsen]

Hi Curtis,

I've been learning my way around Gtk# and Cairo to (among other things) try to understand the Dispose behavior a bit better, but unfortunately haven't gotten back to this specific issue yet. I must say, as a WinForms guy I have found that reading through the Eto.Gtk source code has been one of the best ways to learn Gtk#! (And also learning about NuGet packaging, and Mac deployment, and Visual Studio project templates / plugins... there's really an incredible amount of breadth in this project!)

I had been meaning to chime in on the .NET 4.0 vs 4.5 discussion to mention that my personal preference is to support older frameworks. Up until now I've actually been targeting all of my code to .NET 3.5 (since it is the version that comes with Windows 7 by default, and it seemed to work just fine for everything I've been doing).

As I've been working with Eto.Forms, I've occasionally found it convenient to have a .Net 4.0 version. However, I understand that it's a lot of extra work for you to maintain multiple targets. At least for my work, I think I can live without Mono 2.10 support.

On Mon, Aug 17, 2015 at 9:13 PM, Curtis Wensley notifications@github.com wrote:

Thanks again for all your testing, it really helps!

I did some testing as well, and it seems like my other changes to remove use of obsolete api's are also not compatible with ubuntu 12.04. In order to support it, we'd have to revert some of those changes. I am hoping to work towards removing .net 4.0, so it might make most sense to just target mono 3.2.x/ubuntu 14.04 and greater. This means we should be able to call Dispose() without any conditions to fix this issue.

If you (or anyone) would prefer to support mono 2.10 at least until ubuntu 12.04 is EOL, I would be okay with that, however I am leaning towards removing mono 2.10 support.

— Reply to this email directly or view it on GitHub https://github.com/picoe/Eto/issues/367#issuecomment-132068095.

[cwensley]

Ok, given that no one has complained about the lack of mono 2.10 compatibility of the develop branch thus far, I'll go that direction (not that it's the best indicator, but it's something).

I hear you on supporting older frameworks, I will take that into consideration. Given that .NET 4.0 wasn't standard on any OS (Windows 7 has 3.5 and Windows 8 has 4.5), it doesn't give it much benefit over 4.5 on windows. For the longest time ubuntu only came with mono 2.10 which only supports .NET 4.0.. and now that mono is at least at 3.2 and there are now latest packages for all major distros, it is much less of an issue now.

That being said, I do plan to support .net 4.0 at least for the 2.1 series, and will revisit for each subsequent release. I will ensure to give time for people to voice their opinion before I go forward with it.