FreeGadget causes Unhandled Exception

BlitzMax Forums/MaxGUI Module/FreeGadget causes Unhandled Exception

USNavyFish

(Posted 2009) [#1]

I'm calling FreeGadget(G), where G is a proxy gadget I've created.

The UE come from line 1 of the following block (from win32maxguiex.bmx)

	Method Free()
		If _buttonImageList[0] >= 0 Then ImageList_Destroy(_buttonImageList[0])
		If _iconBitmap Then DestroyIcon( _iconBitmap );_iconBitmap = 0
		_buttonImageList = Null
		Super.Free()
	EndMethod

Any ideas? I've tried using G.Cleanup(), and this yields the same error..

Also, but probably unrelated, I have a timer whose event calls a draw function on my Canvases. At the beginning of the draw function, I use SetGraphics(CanvasGraphics(MyCanvas)). The next line is CLS, which errors out with a UE as well, upon closing the window (which calls the cleanup method mentioned above). I assume the canvas is being wiped out, but then a queued event is trying to setgraphics to that canvas. Shouldn't CLS be checking if a graphics object/driver is set? Regardless of that, I'm hoping that a solution to the first bug will fix this one as well.

Thanks in advance..

Otus	(Posted 2009) [#2]

Looks like it's a TWindowsButton that gets freed. What is your proxy gadget like? Can you show some code?

SebHoll

(Posted 2009) [#3]

The only situtation I could see this throwing an exception is if you are calling FreeGadget() on a button which has already been free'd with a previous call.

USNavyFish

(Posted 2009) [#4]

I agree, somehow the object's being deleted earlier in the program... Originally "SetProxy(Null)" was part of that cleanup code block, but was throwing an exception as well.

I've attached all the code in the boxes at the bottom and broken it down to the relevant sections here.

Proxy Gadget:

Type TGadgetEX Extends TProxyGadget
	Field ParentForm:TForm
End Type

Stored in the 'GadgetList' list within this type:

Type TForm Abstract
	
	Global FormList:TList = CreateList()
	
	Field FormListLink:TLink
	Field GadgetList:TList = CreateList()
		
	Method New()
		FormListLink = FormList.AddLast(Self)
	End Method
	
	Method OnEvent(Event:TEvent) Abstract

       Etc...

Which is deleted with this:

Method FreeForm()
		GCSuspend()
		
		For Local G:TGadgetEX = EachIn GadgetList
			G.ParentForm = Null
			FreeGadget(G)
		Next
		
		FormListLink.Remove()
		FormListLink = Null
		
		GCResume()
	End Method

Calling G.Cleanup() fails, and Calling G.SetProxy(Null) fails, both with UEs.

Perhaps this is also related - but I'm not sure. I have an event handler which checks if event-causing gadgets are of the ProxyGadget type 'TGadgetEX' show above. Many gadgets work, but Canvases don't work the way the other ones do. This is because when a canvas emits an event, the event.Source is a TWindowsPanel... THAT object's source is the Canvas gadget, whose own source (as expected) is the proxy gadget. I'm currently hacking it by iterating through the sources, but that's ugly and probably indicative of an underlying problem. Also probably causing the exception related to canvases mentioned in the first post. Here's the related code. This chunk of code is what I use to wrap all the gadgets (it's actually two functions but I pieced them together for readability).

	Method CreateCanvas:TGadget(x,y,w,h,group:TGadget,style=0)
		Local G: TGadget = MaxGUI.MaxGUI.CreateCanvas:TGadget(x,y,w,h,group:TGadget,style)
               Local GadgetEX:TGadgetEx = New TGadgetEX
		GadgetEX.SetProxy(G)
		GadgetEX.ParentForm = Self
		GadgetList.AddLast(GadgetEX)
		Return GadgetEX.GetProxy()

	End Method

Self is referring to the calling TForm Object

I'm going to post all the code below in the interest of making debugging earlier.. Thanks for your help fellas..

TForm.bmx:

Strict

Import MaxGUI.MaxGUI

Type TGadgetEX Extends TProxyGadget

	Field ParentForm:TForm
		
End Type


Type TForm Abstract
	
	Global FormList:TList = CreateList()
	
	Field FormListLink:TLink
	Field GadgetList:TList = CreateList()
		
	Method New()
		FormListLink = FormList.AddLast(Self)
	End Method
	
	Method OnEvent(Event:TEvent) Abstract
	
	Method SetupGadgetEX:TGadget(Gadget:TGadget)
		Local GadgetEX:TGadgetEx = New TGadgetEX
	
		GadgetEX.SetProxy(Gadget)
		
		GadgetEX.ParentForm = Self
		GadgetList.AddLast(GadgetEX)
		Return GadgetEX.GetProxy()
	End Method

	Method FreeForm()
		GCSuspend()
		
		For Local G:TGadgetEX = EachIn GadgetList
			G.ParentForm = Null
			FreeGadget(G) 
		Next
		
		FormListLink.Remove()
		FormListLink = Null
		
		GCResume()
	End Method
	
	Function EventRouter:Object(ID:Int,Data:Object,Context:Object) Final
		If Not Data Then Return Null
		
		Local Event:TEvent = TEvent(Data)
						
		Local GadgetEX:TGadgetEX 			
	
		Local SourceObject:TGadget = TGadget(Event.Source)
		
		While SourceObject
			If TGadgetEX(SourceObject)
				GadgetEX = TGadgetEX(SourceObject)
				Exit
			Else
				SourceObject = SourceObject.Source
			EndIf
		Wend
		
		If GadgetEX			
			Local SourceForm:TForm = GadgetEX.ParentForm
			Event.Source = GadgetEX.GetProxy()
			SourceForm.OnEvent(Event)	
			Return Null
		EndIf
		
		If TForm(Event.Source) 
			TForm(Event.Source).OnEvent(Event)
			Return Null
		EndIf
					
		Return Data		
	End Function
	
	Function FormsExist:Int()
		Return Not FormList.IsEmpty()
	End Function
	
	
	Method CreateWindow:TGadget( titletext$,x,y,w,h,group:TGadget=Null,style=WINDOW_DEFAULT )
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateWindow:TGadget( titletext$,x,y,w,h,group,style))
	End Method
	
	Method CreateButton:TGadget(label$,x,y,w,h,group:TGadget,style=BUTTON_PUSH)
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateButton:TGadget(label$,x,y,w,h,group,style))
	End Method
	
	Method CreateMenu:TGadget( text$,tag,parent:TGadget,hotKey=0,modifier=0 )
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateMenu:TGadget( text$,tag,parent:TGadget,hotKey,modifier ))
	End Method
	
	Method CreateCanvas:TGadget(x,y,w,h,group:TGadget,style=0)
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateCanvas:TGadget(x,y,w,h,group:TGadget,style))
	End Method
	
End Type

AddHook(EmitEventHook,TForm.EventRouter)

Example Form.bmx:

Import MaxGUI.Drivers
Import "TForm.bmx"

SuperStrict

SeedRnd(MilliSecs())

Type MyForm Extends TForm
	
	Global EVENT_MYFORM_CANVASTIMER:Int = AllocUserEventId()
	
	Field Window:TGadget 
	Field Button:TGadget
	Field Canvas:TGadget
	Field ClickCount:Int = 0
	Field CanvasTimer:TTimer
	Field CanvasHz:Int = 3
	Field TextPos:Float=Rand(0,200)
	
	Field r:Byte =0 
	Field g:Byte =0 
	Field b:Byte =0
	
	Const MENU_EXIT:Int=101
	Const MENU_NEW:Int=102
	Const MENU_ABOUT:Int=103
		
	Function Create:MyForm(title:String,x:Int=-1,y:Int=-1,w:Int=400,h:Int=300)
		Local This:MyForm = New MyForm
		This.Setup(title,x,y,w,h)
		Return This
	End Function
	
	Method Setup(title:String,x:Int,y:Int,w:Int,h:Int)
		Local Style:Int = WINDOW_DEFAULT | WINDOW_CLIENTCOORDS
		If X < 0 Or Y < 0 Then Style = Style | WINDOW_CENTER
		
		Window = CreateWindow(title,x,y,w,h,Desktop(),style)
		SetStatusText(Window,"Click the Button..")
		
		Button = CreateButton("Button",w*(1/3.0),h*(1/3.0),w*(1/3.0),h*(1/3.0),Window)
		Canvas = CreateCanvas(0,h*(2/3.0),w,h*(1/3.0),Window)
		
		Local filemenu:TGadget = CreateMenu("&File",0,WindowMenu(Window))
		CreateMenu"&New Form",MENU_NEW,filemenu
		CreateMenu"E&xit",MENU_EXIT,filemenu,KEY_F4,MODIFIER_COMMAND
	
		Local helpmenu:TGadget =CreateMenu("&Help",0,WindowMenu(Window))
		CreateMenu "&About",MENU_ABOUT,helpmenu
		
		UpdateWindowMenu Window
	
		Local FormEvent:TEvent = CreateEvent(EVENT_MYFORM_CANVASTIMER,Self)
		CanvasTimer = CreateTimer(CanvasHz,FormEvent)
		
	End Method

		
	Method OnEvent(Event:TEvent)
		Select Event.ID
			
		Case EVENT_WINDOWCLOSE
			CloseForm()
		
		Case EVENT_GADGETACTION
			Select Event.Source
			
			Case Button
				ButtonClick()
			
			End Select
			
		Case EVENT_MENUACTION
			Select Event.Data
			
			Case MENU_EXIT
				CloseForm()
				
			Case MENU_NEW
				MyForm.Create("NewForm")
				
			Case MENU_ABOUT
				Notify "Bryan's TForm Example"
				
			End Select
		
					
		Case EVENT_MYFORM_CANVASTIMER
			If Canvas Then DrawCanvas()
			
			
		Case EVENT_MOUSEMOVE
			Select Event.Source
			
			Case Canvas
				r = 0
				g = 255*(Float(Event.X) / GadgetWidth(Canvas))
				b = 255*(Float(Event.Y) / GadgetHeight(Canvas))
				
			End Select
		End Select
	End Method	
	
	Method ButtonClick()
		ClickCount:+1
		SetStatusText(Window,"Clicks: " + ClickCount)
	End Method
	
	Method DrawCanvas()
		SetGraphics(CanvasGraphics(Canvas))
		SetClsColor(r,g,b)
		Cls
			
		TextPos:+ 1
				
		DrawText(String(ClickCount),TextPos Mod GadgetWidth(Canvas),TextPos Mod GadgetHeight(Canvas))
		Flip 0
	End Method
	
	
	Method CloseForm()
		'FreeGadget(Window)
		FreeForm()
	End Method
	
End Type


MyForm.Create("Form1",200,300)
MyForm.Create("Form2")


AddHook EmitEventHook,ProgramEventRouter

While TForm.FormsExist()
	WaitSystem
Wend


Notify "All Forms Have Been Closed!~nProgram Terminating.."


Function ProgramEventRouter:Object(ID:Int,Data:Object,Context:Object)
	If Not Data Then Return Null
	
	Local Event:TEvent = TEvent(Data)
	
	Select Event.ID
	
	Case EVENT_MOUSEMOVE
		'DebugStop()
		
	End Select
	
	Return Data
End Function

Otus	(Posted 2009) [#5]

The problem stems from the fact that you are first freeing a window gadget, thus also freeing its children, and then attempting to free a button that was already freed. You only need to call free for window gadgets, or else start freeing from children up to parents.

EDIT: In any case, is freeing a gadget twice supposed to throw an exception? If that's the case, there should probably be an assert somewhere to check for it and throw a better message...

Grisu

(Posted 2009) [#6]

I get around these errors by checking if the gadget is a null object before trying to free it.

USNavyFish

(Posted 2009) [#7]

Ahh, that makes sense. I'll stick with freeing just the window (the original implementation, as you can see - but it's commented out when I went to introduce the GadgetList w/ cleanup method.

Thanks :) Any clues as to why the canvas issue occurs? Here is the updated code... just run it in debug and close one of the two windows. You'll see the error.

Finally, any ideas as to why the canvas mousemove events are emitted with Event.Soruce as a TWindowsGadget, and not a TGadget would be greatly appreciated!

TForm.bmx:

Strict

Import MaxGUI.MaxGUI

Type TGadgetEX Extends TProxyGadget

	Field ParentForm:TForm
	
	Method CleanUp()
		Super.CleanUp()
		ParentForm = Null
		SetProxy(Null)
	End Method
	
End Type


Type TForm Abstract
	
	Global FormList:TList = CreateList()
	
	Field FormListLink:TLink
		
	Method New()
		FormListLink = FormList.AddLast(Self)
	End Method
	
	Method FreeForm()
		FormListLink.Remove()
		FormListLink = Null
	EndMethod
	
	Method OnEvent(Event:TEvent) Abstract
	
	Method SetupGadgetEX:TGadget(Gadget:TGadget)
		Local GadgetEX:TGadgetEx = New TGadgetEX
		GadgetEX.SetProxy(Gadget)		
		GadgetEX.ParentForm = Self
		Return GadgetEX.GetProxy()
	End Method

	
	Function EventRouter:Object(ID:Int,Data:Object,Context:Object) Final
		If Not Data Then Return Null
		
		Local Event:TEvent = TEvent(Data)
						
		Local GadgetEX:TGadgetEX 			
	
		Local SourceObject:TGadget = TGadget(Event.Source)
		
		While SourceObject
			If TGadgetEX(SourceObject)
				GadgetEX = TGadgetEX(SourceObject)
				Exit
			Else
				SourceObject = SourceObject.Source
			EndIf
		Wend
		
		If GadgetEX			
			Local SourceForm:TForm = GadgetEX.ParentForm
			Event.Source = GadgetEX.GetProxy()
			SourceForm.OnEvent(Event)	
			Return Null
		EndIf
		
		If TForm(Event.Source) 
			TForm(Event.Source).OnEvent(Event)
			Return Null
		EndIf
					
		Return Data		
	End Function
	
	Function FormsExist:Int()
		Return Not FormList.IsEmpty()
	End Function
	
	
	Method CreateWindow:TGadget( titletext$,x,y,w,h,group:TGadget=Null,style=WINDOW_DEFAULT )
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateWindow:TGadget( titletext$,x,y,w,h,group,style))
	End Method
	
	Method CreateButton:TGadget(label$,x,y,w,h,group:TGadget,style=BUTTON_PUSH)
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateButton:TGadget(label$,x,y,w,h,group,style))
	End Method
	
	Method CreateMenu:TGadget( text$,tag,parent:TGadget,hotKey=0,modifier=0 )
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateMenu:TGadget( text$,tag,parent:TGadget,hotKey,modifier ))
	End Method
	
	Method CreateCanvas:TGadget(x,y,w,h,group:TGadget,style=0)
		Return SetupGadgetEX(MaxGUI.MaxGUI.CreateCanvas:TGadget(x,y,w,h,group:TGadget,style))
	End Method
	
End Type

AddHook(EmitEventHook,TForm.EventRouter)

TFormExample.bmx

Import MaxGUI.Drivers
Import "TForm.bmx"

SuperStrict

SeedRnd(MilliSecs())

Type MyForm Extends TForm
	
	Global EVENT_MYFORM_CANVASTIMER:Int = AllocUserEventId()
	
	Field Window:TGadget 
	Field Button:TGadget
	Field Canvas:TGadget
	Field ClickCount:Int = 0
	Field CanvasTimer:TTimer
	Field CanvasHz:Int = 30
	Field TextPos:Float=Rand(0,200)
	
	Field r:Byte =0 
	Field g:Byte =0 
	Field b:Byte =0
	
	Const MENU_EXIT:Int=101
	Const MENU_NEW:Int=102
	Const MENU_ABOUT:Int=103
		
	Function Create:MyForm(title:String,x:Int=-1,y:Int=-1,w:Int=400,h:Int=300)
		Local This:MyForm = New MyForm
		This.Setup(title,x,y,w,h)
		Return This
	End Function
	
	Method Setup(title:String,x:Int,y:Int,w:Int,h:Int)
		Local Style:Int = WINDOW_DEFAULT | WINDOW_CLIENTCOORDS
		If X < 0 Or Y < 0 Then Style = Style | WINDOW_CENTER
		
		Window = CreateWindow(title,x,y,w,h,Desktop(),style)
		SetStatusText(Window,"Click the Button..")
		
		Button = CreateButton("Button",w*(1/3.0),h*(1/3.0),w*(1/3.0),h*(1/3.0),Window)
		Canvas = CreateCanvas(0,h*(2/3.0),w,h*(1/3.0),Window)
		
		Local filemenu:TGadget = CreateMenu("&File",0,WindowMenu(Window))
		CreateMenu"&New Form",MENU_NEW,filemenu
		CreateMenu"E&xit",MENU_EXIT,filemenu,KEY_F4,MODIFIER_COMMAND
	
		Local helpmenu:TGadget =CreateMenu("&Help",0,WindowMenu(Window))
		CreateMenu "&About",MENU_ABOUT,helpmenu
		
		UpdateWindowMenu Window
	
		Local FormEvent:TEvent = CreateEvent(EVENT_MYFORM_CANVASTIMER,Self)
		CanvasTimer = CreateTimer(CanvasHz,FormEvent)
		
	End Method

		
	Method OnEvent(Event:TEvent)
		Select Event.ID
			
		Case EVENT_WINDOWCLOSE
			CloseForm()
		
		Case EVENT_GADGETACTION
			Select Event.Source
			
			Case Button
				ButtonClick()
			
			End Select
			
		Case EVENT_MENUACTION
			Select Event.Data
			
			Case MENU_EXIT
				CloseForm()
				
			Case MENU_NEW
				MyForm.Create("NewForm")
				
			Case MENU_ABOUT
				Notify "Bryan's TForm Example"
				
			End Select
		
					
		Case EVENT_MYFORM_CANVASTIMER
			If Canvas Then DrawCanvas()
			
			
		Case EVENT_MOUSEMOVE
			Select Event.Source
			
			Case Canvas
				r = 0
				g = 255*(Float(Event.X) / GadgetWidth(Canvas))
				b = 255*(Float(Event.Y) / GadgetHeight(Canvas))
				
			End Select
		End Select
	End Method	
	
	Method ButtonClick()
		ClickCount:+1
		SetStatusText(Window,"Clicks: " + ClickCount)
	End Method
	
	Method DrawCanvas()
		SetGraphics(CanvasGraphics(Canvas))
		SetClsColor(r,g,b)
		Cls
			
		TextPos:+ 1
				
		DrawText(String(ClickCount),TextPos Mod GadgetWidth(Canvas),TextPos Mod GadgetHeight(Canvas))
		Flip 0
	End Method
	
	
	Method CloseForm()
		FreeGadget(Window)
		FreeForm()
	End Method
	
End Type


MyForm.Create("Form1",200,300)
MyForm.Create("Form2")


AddHook EmitEventHook,ProgramEventRouter

While TForm.FormsExist()
	WaitSystem
Wend


Notify "All Forms Have Been Closed!~nProgram Terminating.."


Function ProgramEventRouter:Object(ID:Int,Data:Object,Context:Object)
	If Not Data Then Return Null
	
	Local Event:TEvent = TEvent(Data)
	
	Select Event.ID
	
	Case EVENT_MOUSEMOVE
		'DebugStop()
		
	End Select
	
	Return Data
End Function

SebHoll

(Posted 2009) [#8]

Finally, any ideas as to why the canvas mousemove events are emitted with Event.Soruce as a TWindowsGadget, and not a TGadget would be greatly appreciated!

TWindowsGadget is the platform-specific type class used by the Windows implementation of MaxGUI; just like Linux has TFLGadget, and Mac OS X has TCocoaGadget. TWindowsGadget extends TGadget so to all intents and purposes they are the same thing outside of the MaxGUI driver. The object can be cast as both a TWindowsGadget and a TGadget, although if you want your code to work cross-platform you should cast to the more generic type, TGadget. In short, this isn't something you should be worried about - just treat it like a TGadget. This is the beauty of polymorphism.

Any clues as to why the canvas issue occurs? Here is the updated code... just run it in debug and close one of the two windows. You'll see the error.

As for the runtime error, I haven't run the code, but have you remembered to free the timer that you've created to update the canvas. Because if you have free'd the window (and therefore the canvas, as one of its children), the DrawCanvas() code will choke the next time the timer event fires and the tried to draw to a previously free'd canvas. Try adding this before your FreeGadget( window ) line...

StopTimer( CanvasTimer ) 'Frees the canvas timer

I get around these errors by checking if the gadget is a null object before trying to free it.

Grisu, I just thought I should mention that although that will stop one type of memory exception, it wouldn't help in this circumstance. It's because there is a reference to the object stored even after the gadget has been free'd. Most of this time, this would just cause a memory leak, but this time the stray reference is accidentally used to free the gadget a second time, which obviously isn't good.

In any case, is freeing a gadget twice supposed to throw an exception? If that's the case, there should probably be an assert somewhere to check for it and throw a better message...

There is currently no defined behaviour (at least to my knowledge) as to what MaxGUI should do in the case of trying to free a gadget twice.
As a result, sometimes you get runtime errors, whereas other times the function may just fail silently. Perhaps, we should all discuss what behaviour we want in such a circumstance to make sure it is consistent.

Mark Tiffany

(Posted 2009) [#9]

LOL. Exact same question encountered in the GTK web mozilla thread in Brucey's modules, except for treeview nodes being freed twice. All other platforms (inc FLTK) free silently the second time...gtk gets a little annoyed at me continually telling it to free things!

My thinking is that multiple attempts to Free something should *not* error. Ideally BlitzMax would have the concept of warnings, and double freeing would post a warning that the compiler / debugger could choose to ignore via command line parameter...

Brucey

(Posted 2009) [#10]

Perhaps, we should all discuss what behaviour we want in such a circumstance to make sure it is consistent.

Be Assertive :-)

SebHoll

(Posted 2009) [#11]

Argh - I dunno. I think it would be more useful to the programmer to have it flagged immediately when a double free has occurred as it obviously means something has gone wrong with object reference management.

Be Assertive :-)

Hahahaha! Love it! :D

On a serious note, I think you've hit the nail on the head. Check if the gadget has been freed before, and throw a useful error message if it's been previously freed. Unfortunately, this will mean we'll need to get MaxIDE CE working as it should node-freeing wise or it will choke with the official drivers too..

Edit: I can't believe I just laughed at that... I definitely gained some geek points there... :(

Mark Tiffany

(Posted 2009) [#12]

On a serious note, I think you've hit the nail on the head. Check if the gadget has been freed before, and throw a useful error message if it's been previously freed. Unfortunately, this will mean we'll need to get MaxIDE CE working as it should node-freeing wise or it will choke with the official drivers too..

While I personally am happy with this (if not the prospect of revisiting that code!), I am just a little concerned that changing to always assert if a second Free is called might well break a lot of people's code, not just IDE CE.

The problem is that if you have two separate references to a gadget, you can free one and nullify the reference, but unless you can guarantee freeing both references simultaneously, you might want to force a free on the other reference before nullifying in order to prevent leaks.

With normal objects, you could leave this to the GC and just nullify. But with gadgets you (I think) *must* free them in order to release windows resources / remove them from the gui...

Brucey

(Posted 2009) [#13]

a second Free is called might well break a lot of people's code

Implying that a lot of people's code is, in essence, already broken.

Sure, failing silently can be good sometimes... but in reality, the whole idea that you can "free something twice" just seems wrong. In that case, it *must* be a design problem?!?

Mark Tiffany

(Posted 2009) [#14]

Implying that a lot of people's code is, in essence, already broken.

Probably. But I'm always wary of changing something that quietly ignores you doing something stupid into a big flashing neon-lit error.

The specific example in CE IDE is that I added lots of Free's as I was paranoid about memory leaks, and it seems I added too many - essentially doing a Free before every single nulling of a reference.

As a general principle I agree with you, however I'm not completely convinced the design issue isn't in some ways with maxgui - if you null all references to a gadget, the gadget isn't free'd in terms of the GUI elements on screen. If maxgui did this, I could just null out references, free in the one place *I* think I should matter, and let maxgui sort itself out if any objects get orphaned (no references). But if it behaved like this, it would also break a lot of people's code, as you are not required to hold a reference to all your gadgets...and ultimately still obscures a coding issue (that I would be nulling all references before free-ing everything once).

Otus	(Posted 2009) [#15]

Check if the gadget has been freed before, and throw a useful error message if it's been previously freed. Unfortunately, this will mean we'll need to get MaxIDE CE working as it should node-freeing wise or it will choke with the official drivers too..

Ideally, MaxGUI gadgets should not require explicit freeing at all. It should be handled automatically when the object has no more references. So when you want a window to disappear you just Hide it and Null the reference.

However, in case that's not feasible, I think it should throw an exception in debug mode (assert) and preferably silently work in release mode. That way old code would still work in release mode.

Mark Tiffany

(Posted 2009) [#16]

I think that's the thought process I was headed along - just hide and de-reference, and let the GC worry about free-ing. It's what you would do with any other resource in Blitz like an image or whatever...so why not gui elements?

MaxGui is the only Blitz component that still has "Free" in it's vocabulary (3 functions: FreeGadget, FreeMenu, FreeTreeViewNode - why 3 I never know anyway!), so in theory we should get rid!

Issues I can think of:

- maxgui will probably have to ensure each gadget always holds a reference to it's child gadgets, to prevent them being collected away by GC (unless you want to force users to hold a reference to every last gadget?!?)
- people using Free to remove a gadget from a window will now need to use Hide instead - which won't release the memory. They would need to null *all* references, including the maxgui one above...so you might need to be able to not just Hide, but RemoveGadget from a window...effectively just renaming Free!
- people will need to retain a reference to at *least* the top-level gadget (window) for the lifetime of the program (minor issue, and reasonable requiremnt IMHO).

yuck.

Mark Tiffany

(Posted 2009) [#17]

In fact, if you think of "FreeGadget" as what it actually is, which is "RemoveGadgetFromParentGadget", it becomes more and more reasonable to expect it to error if you attempt to RemoveGadgetFromParentGadget twice.

I guess one way of assisting the switchover to assert-ing when you RemoveGadget more than once, would be to have a means of telling if the gadget has already been free-d, so that the lazy coder not wanting to change all their code could change FreeGadget to If not(IsGadgetFree(G)) Then FreeGadget(G). <--yuck again.

Otus	(Posted 2009) [#18]

- maxgui will probably have to ensure each gadget always holds a reference to it's child gadgets, to prevent them being collected away by GC (unless you want to force users to hold a reference to every last gadget?!?)

That's how it works now, isn't it? Every TGadget has a kids:TList field.

- people using Free to remove a gadget from a window will now need to use Hide instead - which won't release the memory. They would need to null *all* references, including the maxgui one above...so you might need to be able to not just Hide, but RemoveGadget from a window...effectively just renaming Free!

Even FreeGadget doesn't release all memory (eg. TGadget objects) until there are no more references. Lurking references are a memory leak. But you are right that it conflicts with holding a reference to children in a gadget.

It would probably require too many API changes for it to happen in MaxGUI.

Mark Tiffany

(Posted 2009) [#19]

That's how it works now, isn't it? Every TGadget has a kids:TList field.

True...so "should" work...and the releasing memory point applies just as well to eg a TImage...

But you are right that it conflicts with holding a reference to children in a gadget.

Yup. I guess at present, the only way to remove the gadget from it's parent list (and hence triggering the GC), is to FreeGadget...

Otus	(Posted 2009) [#20]

It's more like RemoveGadgetFromParentGadgetAndRemoveChildrenFromGadget as the problem in the original posts show.

I guess one way of assisting the switchover to assert-ing when you RemoveGadget more than once, would be to have a means of telling if the gadget has already been free-d, so that the lazy coder not wanting to change all their code could change FreeGadget to If not(IsGadgetFree(G)) Then FreeGadget(G). <--yuck again.

If the Windows implementation already fails on double Free, there might not be many of those lazy coders with this problem? In any case, the code might (should?) already be If g Then FreeGadget g; g = Null, so minimal changes required :p

Mark Tiffany

(Posted 2009) [#21]

If the Windows implementation already fails on double Free, there might not be many of those lazy coders with this problem?

I think it depends on the gadget! TreeViewNodes don't seem to care about double frees on windows, or fltk, but do on gtk!

Brucey

(Posted 2009) [#22]

Only because I didn't expect someone to try to release a gadget more than once :-)

I know... my bad.

I just need to add more... "If handle Then" ... code, to check things haven't already been freed yet.

In wxMax, each wx widget holds a reference to the BlitzMax object (using BBRETAIN in the glue)- so you don't have to keep that reference yourself. Sure, you still have to "Destroy" widgets when you want to release them - but that is the generally expected method in wxWidgets.
However, if you release a container with children, and if the container is designed to release those children, all the BlitzMax object freeing is handled automatically by the GC.

Anyhoo... I guess I'll eventually have a lot more checks in the GTK code to ignore double-freeing attempts.