GtkSharp / GtkSharp

.NET wrapper for Gtk and other related libraries
Other
887 stars 98 forks source link

***MEMORY-ERROR***: APPLICATION[NNNNN]: GSlice: assertion failed: sinfo->n_allocated > 0 #282

Closed muszeo closed 2 years ago

muszeo commented 3 years ago

Seemingly random crash occurring within GTK3 libraries whilst using GtkSharp, GdkSharp, GLibSharp, CairoSharp and PangoSharp wrappers.

I have a largish C# application (~300k lines) that is displaying this crash, usually after loading and closing a largish Gtk Window (150ish widgets) several times. The crash does not necessarily occur directly after the window has been loaded or closed, sometimes several minutes can go by before a crash occurs, but the form is always the same as per title. Despite the volume of widgets, there's nothing particularly exciting going on (notebooks, entries, text areas, combos, colour selectors). Other parts of the app have significant Cairo and Pango drawing which does not appear to trip the crash.

I've tracked down reports of similar errors that stem from the RGBA class, which I am not using directly.

Any ideas?

lytico commented 2 years ago

can you show the code?

muszeo commented 2 years ago

Hey, thanks for your reply. Not specifically, as I'm having trouble trying to track down where it begins, but the complete source code is here. I am wondering if I am not disposing something properly somewhere, and then some sort of race condition is occurring between GTK memory allocation and the .NET GC perhaps (pure guessing here).

muszeo commented 2 years ago

PS -- the issue appears to be far more frequent on Mac OS than it does on either Windows 10 or Ubuntu 20. The issue occurs whether running debug or release code at apparently the same sort of frequency so I don't think it relates to the debugger in any way.

lytico commented 2 years ago

i guess you dispose something what is cross referenced by another class / view ... could be an event too.

BTW, your app isn't easy to debug (that's not a critic!). maybe it would help to have a common superclass for IView (seems always to be a window?), and track calls of Kill. maybe it's sufficient to track Presenter.Kill(IView)

muszeo commented 2 years ago

humm ok, I'll have a dig and see what I can find -- good insight, thanks! I wonder if an event is being held or queued and executed later on or off the main thread. Does GTK do that?

Yes, I need to refactor the views, there's bit of a mess there which doesn't help debugging. I migrated the code base over from the original app which was GTK# GTK2 written using monodevelop about 5-6 years ago, and there is still remnants of the stetic designer in there which need to be weeded out. Having one window super type would help.

lytico commented 2 years ago

another thing i found: there are a lot of code with this pattern:

using(SomeClass _g = new SomeClass()) {
     _g.Dispose();
}

i'm not sure if with this pattern Dispose() isn't called twice.

and i fear that disposing a disposed object is not always implemented correctly ....

so try to remove all calls to Dispose() in using blocks

muszeo commented 2 years ago

Interesting, ok I'll give that a try thanks. Curiously I recently added most of those Disposes () in to ensure that dispose was called, lol. I have had an issue in the past with the GTK# GTK2 version where a Cairo.Context was not being disposed properly in a using block, so I had to add a manual Dispose() call in. I wonder whether in the GTK3 implementation that is no longer necessary. I will give it a try!

lytico commented 2 years ago

about Dispose, see: https://github.com/GtkSharp/GtkSharp/pull/198

muszeo commented 2 years ago

Interesting I note a comment in #198 that identifies that we need to call window.Destroy () ourselves as Dispose () is not doing that any longer. I don't think I'm doing that I'm only disposing. [EDIT: Tried forcing Destroy () when the window closes and disposes but the error still occurs].

muszeo commented 2 years ago

So getting rid of the Dispose () calls from the using blocks doesn't seem to make a difference.

Question: What does the GSlice mechanism get used to allocate? Is it only types that are internal to GTK, or will it get used for objects that are not in GTK such as in the Npgsql libraries I'm using? In theory it shouldn't but I'm clutching at straws here.

lytico commented 2 years ago

that we need to call window.Destroy () ourselves as Dispose () is not doing that

as far as i can see, Window.Dispose calls destroy: Gtk.Window is a Gtk.Bin is a Gtk.Container is a Gtk.Widget, and, Gtk.Widget.Dispose calls Destroy:

https://github.com/GtkSharp/GtkSharp/blob/7c5080506486966bb0e16e14cc380b2aac376e06/Source/Libs/GtkSharp/Widget.cs#L396-L414

lytico commented 2 years ago

Question: What does the GSlice mechanism get used to allocate? Is it only types that are internal to GTK, or will it get used for objects that are not in GTK such as in the Npgsql libraries I'm using?

https://unix.stackexchange.com/questions/281523/tshark-memory-error-6265-gslice-assertion-failed-sinfo-n-allocate https://bugzilla.redhat.com/show_bug.cgi?id=236153 http://technostuff.blogspot.com/2019/03/debugging-issues-related-to-glib.html

....

muszeo commented 2 years ago

Thanks man, very helpful re. destroy. Yeah I've already tried the env variable G_SLICE=always-malloc trick but to no avail. I also tried G_SLICE=debug-blocks to see if that would yield anything that could lead me into the chain of events that causes the error, but there was nothing obvious. Man I thought I'd left these days of memory management issues long behind me lol.

lytico commented 2 years ago

ArcWorx.Continuity.Shapes2D.GtkShapes/Shapes/GtkShapeRenderingStrategyFactory.cs

holds two static objects that are Disposed in other code:


    public class GtkShapeRenderingStrategyFactory : IRenderingStrategyFactory
    {
        #region Private Static Member Variables
        private static Cairo.Context theCairoCtx = null;
        private static Pango.Context thePangoCtx = null;

...

        public static IRenderingStrategyFactory FactoryFor (Cairo.Context cairoCtx, Pango.Context pangoCtx) // Gdk.PangoRenderer pangoRdr)
        {
            if (theFactory == null) {
                theFactory = new GtkShapeRenderingStrategyFactory ();
            }
            theCairoCtx = cairoCtx;
            thePangoCtx = pangoCtx;
            //thePangoRdr = pangoRdr;
            return theFactory;
        }

eg. ArcWorx.Continuity.GtkViews/Diagram2D/Gtk2DObjectPaper.cs

 public void DrawAll (Cairo.Context ctx)
        {
            if (ctx != null) {
                __DrawPaper (ctx);
                __DrawGrid (ctx);
                __DrawRulers (ctx);
                using (Pango.Context _p = CreatePangoContext ()) {
                    __DrawShapes (ctx, _p);
                    _p.Dispose ();
                }

...

        private void __DrawShapes (Cairo.Context ctx, Pango.Context ptx)
        {
           ...
            Colour _c = ColourHelper.InvertedFor (AdjustedColour);
            foreach (IDrawable2DObject _o in theObjects.ZOrderedObjects) {
                if (!theDiagram.ShowFreehand && _o.Model.Element.RepoType.Equals (MetaTypes.RepoTypes.FREEPATH)) {
                    // Do nothing
                } else {
                    if (_o.Model.IsVisible ()) {
                        _o.OutlineColour = _c;
                        (_o as GtkDrawableDiagramElement).Draw (ctx, ptx);
                    }
                }
            }
         ...

ArcWorx.Continuity.GtkViews/Diagram2D/GtkDrawableDiagramElement.cs

        public void Draw (Cairo.Context ctx, Pango.Context ptx)
        {
            theShapeStrategy.SetStrategy (
                GtkShapeRenderingStrategyFactory
                    .FactoryFor (ctx, ptx)
                    .StrategyFor (this, Pg)
            );
muszeo commented 2 years ago

omg, you beauty! Yes, it has to be that. I'll give that a try today and come back to you, but that makes a lot of sense. Fingers crossed!

muszeo commented 2 years ago

Well, I'm going to have to eat my words. I've refactored out those static references and forced the 'rendering strategies' using them to clear their references immediately after drawing is completed. In theory there shouldn't be anything hanging on to references in the factory or the 'rendering strategies' or the Gtk2DObjectPaper. But, it's still popping. I've posted a new code base here.

However -- I've noticed that the act of repeatedly selecting a element on a diagram over and over (without opening any other windows) gradually slows things down. The response time seems to slow up markedly after about 30-40 selections. I'm going to pursue this as a possible avenue leading to the eventual pop.

muszeo commented 2 years ago

Ok, well I think I've tracked this bug down, and I'll post the answer here in case anyone else comes across this.

The issue related to creating new CellRenderer each time a ComboBoxText was refreshed, in the following code:

    private void __PopulateCombo (ComboBoxText combo, string [] values, string set, bool sensitive = true)
    {
        ListStore _m = new ListStore (typeof (string));
        int _s = 0, _i = 0;
        foreach (string _v in values) {
            _m.AppendValues (_v);
            if (_v.ToLower ().Equals (set.ToLower ())) {
                _s = _i;
            }
            _i++;
        }
        combo.Model = _m;
        combo.Active = _s;
        combo.Sensitive = sensitive;
        combo.PackStart (
            StyleFactory.Style.NewComboTextCellRenderer (),
            false
        );
    }

The function StyleFactory.Style.NewComboTextCellRenderer () simply creates a new CellRendererText and attaches a few attributes.

The __PopulateCombo function was used to populate and re-populate several ComboBoxText widgets several times. Each time each Combo is repopulated, the function adds a new CellRenderer to the ComboBoxText. This manifested in the overall performance slowing down each time the function was run, memory usage increasing and eventually the aforementioned MEMORY ERROR.

To resolve this, I added a clause to the function to check whether the ComboBoxText already has a CellRenderer within it, and if so to not add any new ones, as follows:

    private void __PopulateCombo (ComboBoxText combo, string [] values, string set, bool sensitive = true)
    {
        ListStore _m = new ListStore (typeof (string));
        int _s = 0, _i = 0;
        foreach (string _v in values) {
            _m.AppendValues (_v);
            if (_v.ToLower ().Equals (set.ToLower ())) {
                _s = _i;
            }
            _i++;
        }
        combo.Model = _m;
        combo.Active = _s;
        combo.Sensitive = sensitive;
        if (combo.Cells.Length == 0) {
            combo.PackStart (
                StyleFactory.Style.NewComboTextCellRenderer (),
                false
            );
        }
    }

Hope this helps!

lytico commented 2 years ago

I've refactored out those static references and forced the 'rendering strategies' using them to clear their references immediately after drawing is completed.

i woudn't reference any static fields on an object that is going to be disposed. you never know when a deferred call on an event is done, as of the queued drawing of widgets. and, as follow up, never would static reference any widget that could be dynamically disposed in the lifetime of the application.

muszeo commented 2 years ago

Thanks, yes. Maybe the factory doesn't need to cache anything. The creation of new strategies is not a particularly burdensome overhead. Thanks for your help and advice!