GDI resource leak in "custom draw" font handling ?

I've just tracked down a GDI resource leak in my image and one app.  I'm not
sure if the underlying problem is my code, or a bug in Dolphin, anyway here are
the details...

I've been using an idiom for making some elements of a list/tree draw
themselves in bold with code like:

ownerDrawElement: anNMLVCUSTOMDRAW
    (...some condition...) ifTrue:
        [anNMTVCUSTOMDRAW font beBold].

which works fine on the face of it, but turns out to cause a GDI resource leak.
The problem seems to be that Font>>beBold calls Font>>weight: which changes a
setting and then does a 'self free' to cause itself to be re-realised.  As far
as I can tell, the #free is essentially a null-op since the font doesn't think
that it owns its handle, however it appears that Windows thinks that the font
created by NMVCUSTOMDRAW>>font *is* owned.  Hence the resource leak.

I don't know if the above code snippet is how I'm *supposed* to get the effect,
but anyway I've changed it to read more like:

ownerDrawElement: anNMLVCUSTOMDRAW
    (...some condition...) ifTrue:
        [| font |
        font := anNMLVCUSTOMDRAW font.
        font := Font name: font name pointSize: font pointSize.
        font beBold.
        anNMLVCUSTOMDRAW font: font].

which does work, and the resource leak has gone away, but it seems clumsy
somehow...

So, how should I do this ?  And does the font handling actually have a bug in
this area ?

TIA

    -- chris

"Chris Uppal" <[hidden email]> wrote in message
news:40289a30$0$30865$[hidden email]...
> I've just tracked down a GDI resource leak in my image and one app.  I'm
not
> sure if the underlying problem is my code, or a bug in Dolphin, anyway
here are leak.
>....

Well spotted Chris.

> The problem seems to be that Font>>beBold calls Font>>weight: which
changes a
> setting and then does a 'self free' to cause itself to be re-realised.  As
far
> as I can tell, the #free is essentially a null-op since the font doesn't
think
> that it owns its handle, however it appears that Windows thinks that the
font
> created by NMVCUSTOMDRAW>>font *is* owned.  Hence the resource leak.
> ...[snip]...
>....does the font handling actually have a bug in
> this area ?

Yes. There is a general issue with freeing an un-owned object which is then
re-realised, either directly by sending it #free and then using it in some
way that causes it to be lazily re-realized, or indirectly by changing an
attribute (such as the weight of a Font, or the style of a Pen) that causes
Dolphin to free it for it to be re-realised with the new attribute. The
re-realised object still thinks it is unbowed, and so leaks when it is GC'd.

I have attached a hot fix for this. Essentially what the patch does is to
mark any GDI object which Dolphin creates as owned, whether the on first or
subsequent realisations. Some untidiness is then needed to handle stock
objects (which are never owned). We may refactor this a bit for the actual
5.1.4 patch, but this hotfix does at least plug the leak.

Regards

Blair

--------------------

"#1493"

!GraphicsTool methodsFor!

ownedHandle: aHandle
 "Private - Set the handle of the external graphics's tool object
represented and owned by
 the receiver to be the argument."

 self handle: aHandle.
 ownsHandle := true.
 self beFinalizable!

basicRealize
 "Private - Realize (create) the external resource associated with the
receiver, sent from
 the public method, #realize, if not already realized."

 self ownedHandle: self createHandle!

realize
 "Realize (create) the external resource associated with the receiver, but
only if not
 already realized."

 ^self isRealized ifFalse: [self basicRealize]! !
!GraphicsTool categoriesFor: #ownedHandle:!accessing!private! !
!GraphicsTool categoriesFor: #basicRealize!private!realizing/unrealizing! !
!GraphicsTool categoriesFor: #realize!public!realizing/unrealizing! !

!GraphicsTool class methodsFor!

fromOwnedHandle: aHandle
 "Answers an instance of the receiver with aHandle. The handle is owned by
the instance and
 will therefore be freed by it."

 ^(self new)
  ownedHandle: aHandle;
  yourself! !
!GraphicsTool class categoriesFor: #fromOwnedHandle:!instance
creation!public! !

!StockBrush methodsFor!

ownedHandle: aHandle
 "Private - Set the handle of the external graphics's tool object
represented and owned by
 the receiver to be the argument."

 "Implementation Note: Stock objects should never be free'd"

 self handle: aHandle! !
!StockBrush categoriesFor: #ownedHandle:!accessing!private! !

!StockFont methodsFor!

ownedHandle: aHandle
 "Private - Set the handle of the external graphics's tool object
represented and owned by
 the receiver to be the argument."

 "Implementation Note: Stock objects should never be free'd"

 self handle: aHandle! !
!StockFont categoriesFor: #ownedHandle:!accessing!private! !

!StockPen methodsFor!

ownedHandle: aHandle
 "Private - Set the handle of the external graphics's tool object
represented and owned by
 the receiver to be the argument."

 "Implementation Note: Stock objects should never be free'd"

 self handle: aHandle! !
!StockPen categoriesFor: #ownedHandle:!accessing!private! !

Blair,

> I have attached a hot fix for this. Essentially what the patch does is to
> mark any GDI object which Dolphin creates as owned, whether the on first
> or subsequent realisations. Some untidiness is then needed to handle stock
> objects (which are never owned). We may refactor this a bit for the actual
> 5.1.4 patch, but this hotfix does at least plug the leak.

Great.  Thank you very much.  It works a treat.

    -- chris