Ticket #33 (closed defect: fixed)

Opened 5 months ago

Last modified 4 months ago

Memory bug

Reported by: Mike Wey Assigned to: Mike Wey
Priority: major Milestone: Pre-release 9
Component: gtkd - classes Version: TRUNK
Keywords: Cc:

Description (Last modified by okibi)

As outlined in the forum thread (http://dsource.org/forums/viewtopic.php?t=3753) there is a memory bug in gtkD.

The problem is caused by the constructor of gobject.ObjectG which calls objectGSetDataFull("GObject",cast(void*)this); when multiple D objects get allocated for the same gtkStruct* then the pointer to the D object gets overwritten with the newest object, because the key used to store it is always "GObject". this in turn causes gobject.ObjectG.destroyNotify to be called by gtk and the old D object is removed as a GC root and it can be collected.

To fix the problem the constructors for objects that derive from gobject.ObjectG could check if an D object already exists and if so assign it to this. or alternatively this could be done in the functions that return objects.

Attachments

memory-bug.diff (3.2 kB) - added by Mike Wey on 04/30/08 08:01:09.
Diff for the implementation of the constructor option (wrapper only)

Change History

04/30/08 07:52:48 changed by Mike Wey

  • status changed from new to assigned.

Ive implemented the check in the constructor option, this does cause a problem with gtk.Widget.getDrawable and gtk.Widget.getWindow, when getDrawable gets called before getWindow the pointer stored in the gtk struct points to a GdkDrawable? object, when getWindow is called after that the GdkDrawable? is asigned to the GdkWindow?.

For these two functions it's not realy a problem as the gtk.Widget.getDrawable function is redundant as GdkWindow? derives from GkdDrawable?. getDrawable could be removed or it could create a GdkWindow? and return it as a GdkDrawable?.

04/30/08 08:01:09 changed by Mike Wey

  • attachment memory-bug.diff added.

Diff for the implementation of the constructor option (wrapper only)

05/02/08 05:43:22 changed by okibi

  • description changed.

(follow-up: ↓ 4 ) 06/25/08 06:57:33 changed by okibi

Mike, has there been any intention on implementing the change you made? I noticed you left it out of the latest build/wrap you ran (502/504). Is this something that needs to be run soon?

(in reply to: ↑ 3 ) 06/25/08 14:13:07 changed by Mike Wey

Replying to okibi:

Mike, has there been any intention on implementing the change you made? I noticed you left it out of the latest build/wrap you ran (502/504). Is this something that needs to be run soon?

I did wnt to do a few more tests a few months back, before i got ought up in something else. I'll probably add it soon.

06/26/08 06:28:10 changed by okibi

Mike, I ran the build this morning. You can get the test build from ddev at http://www.ratedo.com/ddev/files/gtkd-memfix.zip

06/26/08 06:29:41 changed by okibi

I hope to test this memory fix this morning and report back sometime today. If everything goes as planned, this should get committed tonight. Just to note, the gtkd-memfix.zip build is based on r505.

06/26/08 14:40:48 changed by Mike Wey

tonight for you or for me ;)

So far i've only got the problem mentioned in the first comment.

Here is a small patch for it if you happen to stumble upon it when testing:

  • src/gtk/Widget.d

    old new  
    340340        //      //return new Drawable(cast(GdkDrawable*)(getWidgetStruct().window)); 
    341341        int* pt =cast(int*)getStruct(); 
    342342        pt += 52/4; 
    343         return new Drawable(cast(GdkDrawable*)(*pt)); 
     343        return new Window(cast(GdkWindow*)(*pt)); 
    344344    } 
    345345     
    346346    /** 

  • wrap/APILookupGtk.txt

    old new  
    61006100//      //return new Drawable(cast(GdkDrawable*)(getWidgetStruct().window)); 
    61016101        int* pt =cast(int*)getStruct(); 
    61026102        pt += 52/4; 
    6103         return new Drawable(cast(GdkDrawable*)(*pt)); 
     6103        return new Window(cast(GdkWindow*)(*pt)); 
    61046104    } 
    61056105 
    61066106    /** 

06/27/08 14:09:30 changed by Mike Wey

  • status changed from assigned to closed.
  • resolution set to fixed.

fixed in svn r506/r507