GDI objects leaks

[Patrizio]

Hola Antonio,

I have found some bugs that cause GDI objects leaks.

TTreeVie.prg:

Code: Select all  Expand view

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  Expand view

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  Expand view

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  Expand view

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  Expand view

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

[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

[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.

[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

[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  Expand view

   for nFor := 1 to Len( ::aBitmaps )
      PalBmpFree( ::aBitmaps[ BITMAP_HANDLE ], ::aBitmaps[ BITMAP_PALETTE ] )
   next


That should destroy both the bitmaps and the palettes

[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 ?

[Patrizio]

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  Expand view

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  Expand view

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

[Patrizio]

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

Code: Select all  Expand view

   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  Expand view

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  Expand view

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  Expand view

DeleteObject(aBitmaps[1,1])
DeleteObject(aBitmaps[1,2])

after PalBmpFree the counter be stable.

[Antonio Linares]

Patrizio,

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

We really appreciate your feedback,