HMG4 release method

Moderator: Rathinagiri

User avatar
concentra
Posts: 256
Joined: Fri Nov 26, 2010 11:31 am
Location: Piracicaba - Brasil

Re: HMG4 release method

Post by concentra » Fri Jul 22, 2011 3:36 pm

l3whmg wrote:Hi Mauricio, perhaps I don't understand your code, but I see a lot of release method (I'm referring to the second example)
:o :o :o
Sorry !!!
My mistake, where you see Method Release please interpret as Method CloseQtObject...
:mrgreen:
Post updated...
[[]] Mauricio Ventura Faria

mrduck
Posts: 497
Joined: Fri Sep 10, 2010 5:22 pm

Post by mrduck » Fri Jul 22, 2011 5:05 pm

Carlos Britos wrote: If we do Win_1:Lbl1 := nil then harbour object is destroyed (I'm guess) but the Qt object is there.

Googling a bit it is confirmed that the window is hidden and not deleted at Qt level and this can create a big memory problem.

We can set Qt::WA_DeleteOnClose but keep in mind that it is asyncronuos, memory is released when Qt feels it is necessary and not when the window is closed.

At the same time in ::Release() we must set ::oQTObject := NIL so that we don't map the Qt object anymore.... I'm thinking that this can be enough to delete the Qt object... can you try ?

I think that doing a loop on ::aControls is not necessary at all, probably a
::aControls := NIL // to force deletion
::aControls := {}
should produce the same results.

Can you please produce a little code example that I can use to test this memory problem ?

mrduck
Posts: 497
Joined: Fri Sep 10, 2010 5:22 pm

Post by mrduck » Fri Jul 22, 2011 5:15 pm

Mauricio,
I don't see possible duplicate code1....

It's simply:
METHOD Release() CLASS BASIC
::oQTObject:close() // standard behaviour
RETURN Self

METHOD Release() CLASS MENUITEM/SOUND/TIMER/etc
// do nothing
RETURN Self

I know it seems wrong to have empty methods but it is wrong to call methods on that object firsthand...

Why are we calling :Release() on widgets ? I could not find such call in hmg4 core code... is it for user code ?

Wasn't Release() in hmg3 used only for windows ?

And if we accept to release an object at runtime (instead of just hiding it and let the GC to delete it) shouldn't we also need to remove its DATA ?


Probably I missed the exact point...

User avatar
concentra
Posts: 256
Joined: Fri Nov 26, 2010 11:31 am
Location: Piracicaba - Brasil

Post by concentra » Fri Jul 22, 2011 7:09 pm

I don't see possible duplicate code1....
But we are discussing here add more code to Release() beyond just ::oQtObject:close(), and, if so, duplicated code will rise.
[[]] Mauricio Ventura Faria

User avatar
l3whmg
Posts: 694
Joined: Mon Feb 23, 2009 8:46 pm
Location: Italy
Contact:

Post by l3whmg » Fri Jul 22, 2011 7:46 pm

About me, I never think to "add other code then ::oQtobject:Close()", IMHO because RELEASE must bu used only with QmainWindow, in other word Window HMG object. I have open this topic because I find, within a demo, this code:

Code: Select all

   oPopupFile:Release()
   oPopupHelp:Release()
   oPopupMore:Release()
   oPopupEdit:Release()
   oItemOpen:Release()
   oItemClose:Release()
   oItemMore1:Release()
   oItemMore2:Release()
   oItemMore3:Release()
   oItemCut:Release()
   oItemCopy:Release()
   oItemPaste:Release()
   oItemAbout:Release()
   oItemSave:Release()
   oButton1:Release()
   oButton2:Release()
   oButton3:Release()
   oButton4:Release()
   oButton5:Release()
   oButton6:Release()
   oButton7:Release()
   oButton8:Release()
   oButton9:Release()
   oButton10:Release()
I have given my impression and opinion about some ideas to add code or to force QT event and related problem with the solution.

Cheers
Luigi from Italy
www.L3W.it

Carlos Britos
Posts: 220
Joined: Sat Aug 02, 2008 5:03 pm
Has thanked: 6 times
Been thanked: 2 times

Post by Carlos Britos » Fri Jul 22, 2011 8:11 pm

I'm doing a very basic test, just creating and releasing objects and checking the handles with procesexplorer
mrduck wrote: We can set Qt::WA_DeleteOnClose but keep in mind that it is asyncronuos, memory is released when Qt feels it is necessary and not when the window is closed.


Well if QT take care about this I think there is no problem.

At the same time in ::Release() we must set ::oQTObject := NIL so that we don't map the Qt object anymore.... I'm thinking that this can be enough to delete the Qt object... can you try ?


I tried it but I get a GPF. Just guessing is because we convert the QT pointer to nil but the harbour object is still alive requesting this pointer.

I think that doing a loop on ::aControls is not necessary at all, probably a
::aControls := NIL // to force deletion
::aControls := {}
should produce the same results.

I think is necessary when release some controls and not the whole window.

Below is a basic demo with the method reordered as Luigi has said.
Tracing release method you can see how this works (or not works)

Code: Select all

#include "hmg.ch"
FUNCTION Main
   LOCAL Win_1, LBL1, LBL2, LBL3, oBTN, tmr, LBL4
   Define Window Win_1 ;
      At 10, 10 ;
      Width 650 ;
      Height 400 ;
      Main

      Define Main Menu
         Define Popup "test"
            Item "Release LBL1"   Action { || Win_1:LBL1 := NIL,;
                                                   WIN_1:LBL1:Show(),;
                                                   WIN_1:LBL1:Value( '********' ) }

            Item "Release LBL2:LBL3"   Action { || LBL2:LBL3:Release(),;
                                                   LBL2:LBL3:Show(),;
                                                   LBL2:LBL3:Value( 'ooooooooooo' ) }

            Item "Release LBL4"   Action { || LBL4:Release(), LBL4:oQTObject := NIL, LBL4 := NIL }

         End Popup
      End Menu

      With Object oBTN := Button():New( "oBTN" )
         :Row     := 40
         :Col     := 290
         :Width   := 150
         :Caption := 'Open Standard Window'
      End With
      oBTN:OnClick := { || Button1Click() }

      With Object LBL1 := label():New( "LBL1" )
         :value   := 'LBL1 parent of Win_1'
         :Row     := 40
         :Col     := 100
         :Width   := 200
      End
      LBL1:OnClick :=  { || Win_1:LBL1:release() }

      With Object LBL2 := label():New( "LBL2" )
         :value   := 'LBL2 parent of Win_1'
         :Row     := 80
         :Col     := 100
         :Width   := 200
      End
      LBL2:OnClick  :=  { || Win_1:LBL2:release() }

      With Object LBL3 := label():New( "LBL3" )
         :value   := 'LBL3 parent of LBL2'
         :Row     := 120
         :Col     := 100
         :Width   := 200
         :OnClick :=  { || LBL3:release() }
      End
      LBL3:Parent   := LBL2

      With Object  tmr := Timer():New( "tmr" )
         :Interval     := 1000
      End
      tmr:Parent   := LBL3

      With Object LBL4 := label():New( "LBL4" )
         :value   := 'LBL4'
         :Row     := 160
         :Col     := 100
         :Width   := 200
         :OnClick :=  { || LBL4:release() }
      End

   End Window
   Activate Window Win_1
   Win_1:Release()

   RETURN 0

/*----------------------------------------------------------------------*/

PROCEDURE Button1Click()
   LOCAL oStd , oBTN

   With Object oStd := Window():New()
      :Row     := 10
      :Col     := 10
      :Width   := 600
      :Height  := 300
      :Title   := 'Nice OOP Standard Window!!!'
      :Type    := WND_STANDARD
      :OnInit  := { || oStd:Center() }

      With Object oBTN := Button():New()
         :Row      := 40
         :Col      := 40
         :Caption  := 'Close Window'
      End With
      oBTN:OnClick  := { || oStd:Release() }

   End With

   oStd:Activate()

   oStd:Release()

   RETURN

/*----------------------------------------------------------------------

METHOD Release() CLASS BASIC

   LOCAL i

   // Release all child objects in it.
   // Recursive !
   FOR i := 1 TO Len( ::aControls )
      IF hb_IsObject( ::aControls[ i ] )
         ::aControls[ i ]:Release()
      ENDIF
   NEXT

   // Remove from the parent pool control
   FOR i := 1 TO Len( ::oParent:aControls )
      IF hb_IsObject( ::oParent:aControls[ i ] )
         IF ::oParent:aControls[ i ]:Name == ::Name
            ::oParent:aControls[ i ] := Nil
            // ::oParent:oQTObject := NIL   //        => GPF
         ENDIF

      ENDIF
   NEXT

   // Some QT classes do not have close()
   IF ! ( ::cClass == "MENUITEM"   .OR. ;
          ::cClass == "SOUND"      .OR. ;
          ::cClass == "TIMER"      .OR. ;
          ::cClass == "LAYOUTBOX"  .OR. ;
          ::cClass == "LAYOUTFORM" .OR. ;
          ::cClass == "LAYOUTGRID" )

      // release QT object
      ::oQTObject:close()
      // ::oQTObject := NIL   //        => GPF

   ENDIF

   RETURN NIL                                                     */
Regards/Saludos, Carlos (bcd12a)

Carlos Britos
Posts: 220
Joined: Sat Aug 02, 2008 5:03 pm
Has thanked: 6 times
Been thanked: 2 times

Post by Carlos Britos » Fri Jul 22, 2011 8:12 pm

Hi
l3whmg wrote:About me, I never think to "add other code then ::oQtobject:Close()", IMHO because RELEASE must bu used only with QmainWindow, in other word Window HMG object. I have open this topic because I find, within a demo, this code:

Code: Select all

   oPopupFile:Release()
   oPopupHelp:Release()
   oPopupMore:Release()
   oPopupEdit:Release()
.....
   oButton10:Release()
I have given my impression and opinion about some ideas to add code or to force QT event and related problem with the solution.

Cheers
Because of this I'm adding the for next in release method
Regards/Saludos, Carlos (bcd12a)

Carlos Britos
Posts: 220
Joined: Sat Aug 02, 2008 5:03 pm
Has thanked: 6 times
Been thanked: 2 times

Post by Carlos Britos » Fri Jul 22, 2011 8:16 pm

l3whmg wrote:IMHO because RELEASE must bu used only with QmainWindow, in other word Window HMG object.
Cheers
I'm not agree, each object must have a release method, One for all as far as is possible.
Regards/Saludos, Carlos (bcd12a)

mrduck
Posts: 497
Joined: Fri Sep 10, 2010 5:22 pm

Post by mrduck » Fri Jul 22, 2011 9:04 pm

Carlos Britos wrote:

Code: Select all

#include "hmg.ch"
FUNCTION Main

Please add here a hbqt_errorsys() call and you will see some harbour errors...

mrduck
Posts: 497
Joined: Fri Sep 10, 2010 5:22 pm

Post by mrduck » Fri Jul 22, 2011 11:03 pm

Carlos, I still have some problems trying to focus on this problem. I "feel" there is something not really clear...

You want to be able to delete a HMG4 object and automatically delete the underlaying hbQt and expecially Qt objects. In every message/thread/sample I find on the web, this is NOT POSSIBLE in Qt. Memory management is done by Qt with its own rules. It took more than one year to understand what happens and how to interface this in hbQt ! Endless sleepless nights...

The only way to ask Qt to delete the object is using the Qt::WA_DeleteOnClose. And you are just asking Qt to "please, can you delete the object when you want to do it..., really thanks...."

That said, your sample code looks strange to me..... let me explain

(I separate with letters so that we can differentiate the discussion)

A) Parenting

I don't understand why you are setting the parents in this way.... are these parenting useful in a application ?

:parent has good code, for examply it correctly handles the removal from the parent both as DATA member and in ::aControls array, something :release() in the various forms doesn't do correctly.
What :parent() doesn't do is Qt parenting !!!!!

Have a look at Label:New(). Creating the QLabel we set a Qt parenting. ::parent() doesn't update this parenting.

So after the line
LBL3:Parent := LBL2
the HMG parenting is one, the Qt parenting is another one and LBL3 is still a child of the window....

You may change the code to

Code: Select all

With Object LBL3 := label():New( "LBL3", LBL2 )
but the label won't show... a label inside a label ?!?!?!?


B) why qt object doesn't get deleted

Code: Select all

   // Remove from the parent pool control
   FOR i := 1 TO Len( ::oParent:aControls )
      IF hb_IsObject( ::oParent:aControls[ i ] )
         IF ::oParent:aControls[ i ]:Name == ::Name
            ::oParent:aControls[ i ] := Nil
            // ::oParent:oQTObject := NIL   //        => GPF
         ENDIF
      ENDIF
   NEXT
Doesn't the ::oParent:aControls[ i ] := Nil cuts the rope we are climbing ? Well, actually it cuts one of the THREE ropes: we have one HMG4 object that is pointed by 3 variables, one is the LOCAL variable, the other is in the aControls array, the last is the DATA member.

Code: Select all

METHOD NEW CLASS CONTROL
....
IF hb_isobject( ::oParent ) == .T.
   AADD( ::oParent:aControls , Self )
   ::oParent:AddData( ::cName , Self )
ENDIF
LBL1 is accessible by the LOCAL LBL1, by Win_1:lbl1 and ::oParent:aControls[ <index of lbl1> ] (BTW, why don't we switch to a hash???)

So to REALLY permit Qt to DELETE a Qt object we should set THREE NILs...
The code for two of them in in :parent(), the third is just a lbl1 := NIL

Now the GC should be able to take care of the situation....


C) main and child windows

Now that we should have found why the Qt objects doesn't get deleted in this specific situation, I'd like to understand WHY you should want to delete them in the main window.

hbide at startup creates all the windows, panes, tabs and just switches them on/off - is is slower at startup but it results in a quicker UI experience for the user at runtime. Skype uses Qt. It has one "main form" (the list of contacts, with at couple of tabs, etc, etc) there are created at startup and never deleted and when necessary (for chats, calls etc) a new window is opened and then closed.
The main form is built to stay "on" for all the life of the program and anyway in skype and hbide the objects are not "parented" by the main form but by layouts, panes, tables etc. There should ALWAYS be a "central widget" in a QMainWindow if you don't want to have problems with the layout !!!!

If you want to use Qt setParent (and we saw we didn't call it in point A) read what the Qt docs says:

Code: Select all

Warning: It is very unlikely that you will ever need this function. If you have a widget that changes its content dynamically, it is far easier to use QStackedWidget.
That said I will concentrate the attention on the Button1Click() function since in a normal business program you will use a lot of "child" windows, can they be QDialog with tons of hmg objects inside...

The real question we should ask is: is the memory and the Qt objects freed when we call functions like Button1Click and they return the control to the calling function (the oStd LOCAL variable goes out of scope) ?

It doesn't happen with Button1Click because.... please read following point before doing tests

D) how to close a window

From QWidget:close() description: (close() is called by window:release() )

Code: Select all

First it sends the widget a QCloseEvent. The widget is hidden if it accepts the close event. 
This means that if we add the msginfo line in your demo

Code: Select all

oStd:Activate()
msginfo( "button1click about to return" )
oStd:Release()
we will NEVER see that msginfo !!!
The window gets hidden, memory is NOT released (hmg/hbQt/Qt) and when we press Win_1:oBtn a NEW window is created and shown !

Is it possible to solve this ? Of course :-) But unfortunately window.prg code is flawed...

In your sample code the oStd window is created of type WND_STANDARD. In this way you may open dozens of oStd indipendent windows. When windows type is not WND_MAIN the windows is run by hmg using QEventLoop:exec( 0 ) and this means that we must CLOSE (and not hide) the window with :exit()

Please note how the situation is handled in OnRelease method for WND_MAIN e WND_MODAL window types, where we force an exit() or quit().

Now look at the Window:Release() method

Code: Select all

METHOD Release class WINDOW
   IF ::nType == WND_MODAL
      ::oEventLoop:Exit( 0 )
   ENDIF
   Super:Release()  // it calls :close()
RETURN NIL
When window type is WND_STANDARD it doesn't stop the eventloop....

Since WND_MAIN is the only window type run in a different way (we should check all the windows types) I changed the if in:

Code: Select all

IF ::nType != WND_MAIN
   ::oEventLoop:Exit( 0 )
ENDIF
and now everything works, the window is closed, the msginfo shown and when oStd goes out of scope used memory (hmg/hbQt/Qt) should be correctly recovered.

Post Reply