Page 1 of 1

GDI objects leaks

Posted: Wed Jan 23, 2008 4:05 pm
by Patrizio
Hola Antonio,

I have found some bugs that cause GDI objects leaks.

TTreeVie.prg:

Code: Select all

METHOD SetColor( nClrText, nClrPane ) INLINE ;
Super:SetColor( nClrText, nClrPane ), TVSetColor( ::hWnd, nClrText, nClrPane )
TVSetColor increment by two the GDI objects (two brushes) in use without decrement when destroy the TreeView. If you don't use custom color for the treeview you can don't call TVSetColor so I've change in:

Code: Select all

METHOD SetColor( nClrText, nClrPane ) CLASS TTreeView
   Super:SetColor( nClrText, nClrPane )                                                   
   IF !Empty(::hWnd) .AND. ( nClrText != ::oWnd:nClrText .OR. nClrPane != GetSysColor(COLOR_WINDOW) )
      TVSetColor( ::hWnd, nClrText, nClrPane )                                            
   ENDIF              
RETURN NIL
If you use a ImageList with the TreeView it's not destroyed when you destroy the TreeView so I've add the method Destroy:

Code: Select all

METHOD Destroy() CLASS TTreeView
   IF !Empty(::oImageList)                                                                
      ::oImageList:End()                                                                  
   ENDIF                                                                                  
RETURN Super:Destroy()
BtnBmp.prg

In the method LoadBitmaps there's no call to FreeBitmaps, so if you change the bitmaps at run-time it will open the new bitmaps without closing the others.

xBrowse.prg

Sometimes the method End isn't called when you close the dialog. I add a inline method destroy:

Code: Select all

METHOD Destroy() INLINE ::End()
In the End method of TXBrwColumn class there is the DeleteObject of the bitmaps you used. But there isn't the DeleteObject of the bitmap's palette.

Code: Select all

DeleteObject( ::aBitmaps[ nFor, BITMAP_PALETTE ] )
In the SetArray method if you use lAutoOrder two bitmaps (created by FwBmpAsc() and FwBmpDes()) are added to each column. It is not necessary, you can create the two bitmap handles when create the xBrowse and use the same for each column or create when your application start and use the same for each column of each xbrowse.


Thanks
Patrizio

Posted: Wed Jan 23, 2008 10:07 pm
by Antonio Linares
Patrizio,

>
TVSetColor increment by two the GDI objects (two brushes) in use without decrement when destroy the TreeView.
>

How do you know this ? How have you checked it ? Thanks

Posted: Wed Jan 23, 2008 10:12 pm
by Antonio Linares
Patrizio,

>
If you use a ImageList with the TreeView it's not destroyed when you destroy the TreeView so I've add the method Destroy:
>

We can not destroy the oImageList because it may be in use from another TreeView, or other controls that use imagelists.

Thats why we need to specifically call oImageList:End() when we are sure that no others controls are using it

We could enhance that class to have an use counter, like we do with brushes and fonts, then we could reuse them and your code would be ok.

Posted: Wed Jan 23, 2008 10:18 pm
by Antonio Linares
Patrizio,

>
Sometimes the method End isn't called when you close the dialog. I add a inline method destroy:

METHOD Destroy() INLINE ::End()
>

I think the right solution is to rename the method End() as method Destroy() in class TXBrowse. It should be called Destroy, not End.

The right calling sequence is: End() --> Destroy(). Your solution could result into a dangerous loop

Posted: Wed Jan 23, 2008 10:22 pm
by Antonio Linares
Patrizio,

>
In the End method of TXBrwColumn class there is the DeleteObject of the bitmaps you used. But there isn't the DeleteObject of the bitmap's palette.
>

In METHOD End() CLASS TXBrwColumn there is this code:

Code: Select all

   for nFor := 1 to Len( ::aBitmaps )
      PalBmpFree( ::aBitmaps[ BITMAP_HANDLE ], ::aBitmaps[ BITMAP_PALETTE ] )
   next
That should destroy both the bitmaps and the palettes

Posted: Wed Jan 23, 2008 10:25 pm
by Antonio Linares
Patrizio,

>
In the SetArray method if you use lAutoOrder two bitmaps (created by FwBmpAsc() and FwBmpDes()) are added to each column. It is not necessary, you can create the two bitmap handles when create the xBrowse and use the same for each column or create when your application start and use the same for each column of each xbrowse.
>

Yes, you are right, but again we face the same reuse problem: How to know when those bitmaps should be released ? How to know if other columns are using them ?

Posted: Thu Jan 24, 2008 9:24 am
by Patrizio
Antonio Linares wrote:Patrizio,

>
TVSetColor increment by two the GDI objects (two brushes) in use without decrement when destroy the TreeView.
>

How do you know this ? How have you checked it ? Thanks
I use MSDN GDIndicator e GDIUsage : http://msdn.microsoft.com/msdnmag/issue ... /GDILeaks/

In this case GDIndicator show you the type of gdi objects used by an application.

I've check it with this sample:

Code: Select all

PROC Main()
   LOCAL oDlg, n
   FOR n := 1 TO 5
      InitDialogGDI()
      SysRefresh()
      HB_GCall()
      MsgInfo("GDI objects:" + AllTrim(Str(GetGDIObjects(),7)))
   NEXT
RETURN

PROC InitDialogGDI()
   LOCAL oDlg, oTree
   oDlg  := TDialog():New(,,,,,"DLG_GDI")
   oTree := TTreeView():ReDefine(1190,oDlg,CLR_BLUE,CLR_YELLOW,.T.,"l0")
   oDlg:Activate(,,,.F.,{||.T.},.T.,,,,)
   oDlg:End()
RETURN

#pragma BEGINDUMP

#include <hbapi.h>
#include <Windows.h>
#include <WinUser.h>
#include <ClipApi.h>

HB_FUNC( GETGDIOBJECTS )
{
   hb_retnd( GetGuiResources( GetCurrentProcess(), 0 ) );
}

#pragma ENDDUMP

Code: Select all

DLG_GDI DIALOG DISCARDABLE 0, 0, 616, 428
STYLE DS_CENTER|WS_THICKFRAME|WS_CAPTION|WS_SYSMENU|WS_MINIMIZEBOX|WS_MAXIMIZEBOX|WS_VISIBLE
CAPTION "GDI"
FONT 8, "MS Sans Serif"
BEGIN
  CONTROL "", 1190, "SysTreeView32", TVS_HASBUTTONS|TVS_HASLINES|TVS_LINESATROOT|TVS_EDITLABELS|TVS_SHOWSELALWAYS|TVS_NOTOOLTIPS|TVS_FULLROWSELECT|WS_BORDER|WS_TABSTOP, 8, 36, 220, 312
END

Posted: Thu Jan 24, 2008 10:26 am
by Patrizio
Antonio Linares wrote:In METHOD End() CLASS TXBrwColumn there is this code:

Code: Select all

   for nFor := 1 to Len( ::aBitmaps )
      PalBmpFree( ::aBitmaps[ BITMAP_HANDLE ], ::aBitmaps[ BITMAP_PALETTE ] )
   next
That should destroy both the bitmaps and the palettes

Code: Select all

PROC Main()
   LOCAL aBitmaps := {}
   LOCAL aBmp1, aBmp2, aBmp3, n
   FOR n := 1 TO 5
      aBitmaps := {}
      aAdd(aBitmaps,PalBmpLoad( "TEST_GDI" ))
      PalBmpFree( aBitmaps[1,1], aBitmaps[1,2] )
      SysRefresh()
      HB_GCall()
      MsgInfo("GDI objects:" + AllTrim(Str(GetGDIObjects(),7)))
   NEXT
RETURN

#pragma BEGINDUMP

#include <hbapi.h>
#include <Windows.h>
#include <WinUser.h>
#include <ClipApi.h>

HB_FUNC( GETGDIOBJECTS )
{
   hb_retnd( GetGuiResources( GetCurrentProcess(), 0 ) );
}

#pragma ENDDUMP

Code: Select all

TEST_GDI    BITMAP "C:\\FWH\\BITMAPS\\16x16\\Copy2.bmp"
After the first the counter of gdi objects increments by one for each iteration (the first count the gdi objects of the msginfo).

If you add

Code: Select all

DeleteObject(aBitmaps[1,1])
DeleteObject(aBitmaps[1,2])
after PalBmpFree the counter be stable.

Posted: Thu Jan 24, 2008 10:35 am
by Antonio Linares
Patrizio,

Many thanks for your information. We are going to fix it

We really appreciate your feedback,