Is there a memory leak in my code?

[tmighty2]

Hello!

I have some code to take a screenshot of the bitmap.
After like 300 runs, the VB6 IDE says good-bye with some graphic error. I know this behaviour from memory leaks.

I have looked through my screencode-code again and again, but I don't see any memory leak.

Can somebody tell me if there indeed isn't any leak? At the end of the code I do something else (using LaVolpe's c32bppDIB class, but I don't think that it doesn't have any errors, so I will leave this out for a start).

Thank you!

Option Explicit

Public c As c32bppDIB

Private hDCSrc As Long
Private hDCMemory As Long
Private hBmp As Long
Private hBmpPrev As Long

Private m_bCleant As Boolean

Public Sub TakeScreenshot()

    Dim WndHandle&
    WndHandle = GetDesktopWindow

    'Get Window Size
    Dim WidthSrc As Long
    Dim HeightSrc As Long
    Dim rc As RECT
    GetWindowRect WndHandle, rc
    WidthSrc = rc.Right - rc.Left
    HeightSrc = rc.Bottom - rc.Top
    
    'Get Window  device context
    hDCSrc = GetWindowDC(WndHandle)
    
    'create a memory device context
    hDCMemory = CreateCompatibleDC(hDCSrc)
    
    'create a bitmap compatible with window hdc
    hBmp = CreateCompatibleBitmap(hDCSrc, WidthSrc, HeightSrc)
    
    'copy newly created bitmap into memory device context
    hBmpPrev = SelectObject(hDCMemory, hBmp)

    'copy window window hdc to memory hdc
    Call BitBlt(hDCMemory, 0, 0, WidthSrc, HeightSrc, hDCSrc, 0, 0, vbSrcCopy)
      
    'Get Bmp from memory Dc
    hBmp = SelectObject(hDCMemory, hBmpPrev)

    'copy window window hdc to memory hdc
    Call BitBlt(hDCMemory, 0, 0, WidthSrc, HeightSrc, hDCSrc, 0, 0, vbSrcCopy)
      
    Set c = New c32bppDIB

    Dim b As Boolean
    b = c.LoadPicture_ByHandle(hBmp)
    Debug.Assert b
    
End Sub
Public Sub CleanUp()

    hBmp = SelectObject(hDCMemory, hBmpPrev)
    DeleteObject hBmp
    
    DeleteDC hDCMemory
    
    Set c = Nothing
    
    m_bCleant = True

End Sub
Private Sub Class_Terminate()

    If Not m_bCleant Then
        Me.CleanUp
    End If

End Sub

And this is how I call it:

Private Sub btnTest_Click()

    Dim l&
    For l = 1 To 50

        Dim cSS As clsScreenshot
        Set cSS = New clsScreenshot
        cSS.TakeScreenshot
        cSS.c.SaveToFile_PNG "d:\blatest1.png", False
        cSS.CleanUp
        Set c = Nothing
        
    Next

End Sub

[Thierry69]

You should put the code

hBmp = SelectObject(hDCMemory, hBmpPrev)
DeleteObject hBmp
Call DeleteDC(hDCMemory)

At the end of you procedure

[Zvoni]

I think that's the classic mistake understanding how the Terminate-Event works.
The Terminate-Event fires only when the last instance of the class is destroyed!

[dilettante]

I think that's the classic mistake understanding how the Terminate-Event works.
The Terminate-Event fires only when the last instance of the class is destroyed!

Um.. no.

What you might be trying to say is that the event is raised when the last counted reference to a class instance is released.

[tmighty2]

I am setting the class instance to "nothing" when I'm done.

Like that:

      Dim c As New clsScreenshot
      c.TakeScreenshot
      Set c = Nothing

Is that sufficient to actually terminate the class?

[tmighty2]

@Thierry69

I'm doing something from "outside", so I can not call this clean-up code at the end of the procedure:

I call a public sub of the class (which I have left out for readability... it features LaVolpe's c32bppDIB class), so I can not call this at the end of the procedure.

What I'm actually doing is this:

Private Sub btnTest_Click()

    Dim c As clsScreenshot
    Set c = new clsScreenshot
    c.TakeScreenshot 
    c.c32bppDIBsuite.SaveToFile_PNG "d:\sometest1.png", False
    
    Set c = Nothing
    
Exit Sub

[LaVolpe]

There are a couple things to note:

1. When calling GetWindowDC, call ReleaseDC later. See MSDN documentation

2. You are leaking a bitmap.

In TakeScreenshot, you have these lines
- hBmp is created
- hBmp is selected into DC and previous bmp placed in hBmpPrev (so far, ok)
- hBmp is unselected from DC (ok, normal & hBmp is no longer in the DC)

But hBmp is never destroyed. The class terminate does not destroy it either. When that event is triggered, hBmp is already out of the DC, you assign it to the DC's current Bmp and destroy that instead -- double whammy; not only are you not destroying something you created, you are destroying something you didn't create

Recommendation: Do not hold onto hBmp. Destroy it when done with it during TakeScreenshot

[Elroy]

Is that sufficient to actually terminate the class?

tmighty2,

Here's the language I use. We've got classes, we've got objects, and we've got object variables that reference those objects.

Now, I think of a class as a template for creating an instance (i.e., instantiating) an object. The actual machine (or p) code is re-entrant, so it really just stays the same. However, a unique set of internal variables is created for each instance of an object we create. So, here's the tricky part #1: We can use a class to instantiate more than one object, which we often do.

Ok, moving on. When we instantiate an object from some class, we typically have some variable that references that new object. And here's tricky part #2. We often have multiple object variables that reference the same instantiated object. Internally, each instantiated object has a reference count. If things are done correctly, that object's internal reference count should equal the number of object variables that reference that object.

So, to answer your question, an object's terminate event fires when that object's reference count goes to zero. Or, said a bit differently, an object's terminate event fires when the last object variable referencing it is set to "Nothing". And, to address a flaw in your above quote, classes aren't terminated ... only objects are terminated, and only when there are no longer any object variables referencing it.

Here's an example that attempts to illustrate the concepts. I just put a single class (Class1) into a new project. The Form1 is also a class, but I just use some code in the Form_Load to illustrate the above concepts with Class1:

Option Explicit

Private Sub Form_Load()
    Dim Object1_Variable1 As Class1
    Dim Object1_Variable2 As Class1
    Dim Object1_Variable3 As Class1
    '
    Dim Object2_Variable1 As Class1
    Dim Object2_Variable2 As Class1
    Dim Object2_Variable3 As Class1



    Set Object1_Variable1 = New Class1          ' Instantiate Object1 from Class1.
    Set Object1_Variable2 = Object1_Variable1   ' Create a second reference to Object1.
    Set Object1_Variable3 = Object1_Variable2   ' Create a third reference to Object1.

    Set Object2_Variable1 = New Class1          ' Instantiate Object2 from Class1.
    Set Object2_Variable2 = Object2_Variable1   ' Create a second reference to Object2.
    Set Object2_Variable3 = Object2_Variable1   ' Create a third reference to Object2.


    Set Object1_Variable1 = Nothing             ' Object1 still exists, but this reference is removed.
    Set Object1_Variable2 = Nothing             ' Object1 still exists, but this reference is removed.
    Set Object1_Variable3 = Nothing             ' Object1 is un-instantiated, and the "terminate" event will immediately fire before this code continues execution.


    ' When this code hits the "End Sub" (or an Exit Sub), all local variables will be freed.  Therefore, the Object2 will be un-instantiated, and it's "terminate" event will fire.
End Sub

Now, notice in the above that we only have one class. But we have two objects. And, for each of those objects, we have three references each (object variables referencing the objects).

Maybe that'll help. So, to state again, an object is un-instantiated (with it's terminate event immediately fired) when its internal reference count goes to zero. And, if things are done correctly, that should correspond to when the last variable referencing that object is set to Nothing (which also happens when the variable is destroyed from its lifetime ending).

Good Luck,
Elroy

[tmighty2]

@LaVolpe

Thank you, but am I not destroying hBmp here?

-->

    hBmp = SelectObject(hDCMemory, hBmpPrev)
    DeleteObject hBmp

I have changed my initial post to include just everything, I think that becomes important now.

[LaVolpe]

@LaVolpe

Thank you, but am I not destroying hBmp here?

No. In TakeScreenshot, you already have this line: hBmp = SelectObject(hDCMemory, hBmpPrev). That is removing hBmp from the DC and putting the previous one back into the DC. You then call the same line again in cleanup. Now, hBmp is the previous bitmap. You have lost/leaked the hBmp you originally created.

Remove that line from cleanup and that leak should go away. You can verify this easily, after each of those 2 lines are called:
1. Print out the value of hBmp you created in TakeScreenshot
2. Print out the value of hBmp when you call cleanup

To try to show what's happening...

In Public Sub TakeScreenshot()

hBmp = CreateCompatibleBitmap(hDCSrc, WidthSrc, HeightSrc)

hBmp is YourBmp, hBmpPrev is 0

hBmpPrev = SelectObject(hDCMemory, hBmp)

hBmp is YourBmp, hBmpPrev is OtherBmp

hBmp = SelectObject(hDCMemory, hBmpPrev)

hBmp is YourBmp, hBmpPrev is OtherBmp

In Public Sub CleanUp()

hBmp = SelectObject(hDCMemory, hBmpPrev)

hBmp is OtherBmp, hBmpPrev is Undefined

Note: Cleanup line could actually fail and hBmp would be zero. It could fail because hBmpPrev may no longer be a valid bitmap, it is possible it may have already been destroyed by its creator (the Desktop)

Your class probably should be reworked. The leak can occur if you call TakeScreenShot more than once without calling Cleanup between the calls. And in that case, why not just clean up after the screenshot? There is no point in keeping the bitmap around. Maybe keep the DC you created until the class terminates, but don't see a reason to keep the bitmap -- just a leak waiting to happen.

Last but not least, once you are happy with your code, compile the project and run it in 150% to 175% DPI. You may be disappointed with the results unless you manifest the project as DPI aware. I try to remember to provide this note whenever I see screenshot-related code.

[tmighty2]

Thank you for the great explanation.

I have reworked the class now, and I inserted statements to tell me that value of hBmp.
Even after CleanUp, it stays a longer number (=not 0). Does that tell me that deleting it failed?

I never call "TakeScreenshot" more than once.

The huge leak went away, but I still lose 2 GDI objects per class instance.

Is there anything in the part of the code that I wrote that would still leak? I don't see it.

Here is the new, improved version of my class:

Option Explicit

Public c As c32bppDIB

Private hDCSrc As Long
Private hDCMemory As Long
Private hBmp As Long
Private hBmpPrev As Long

Private m_bCleant As Boolean

Public Sub TakeScreenshot()

    Dim WndHandle&
    WndHandle = GetDesktopWindow

    'Get Window Size
    Dim WidthSrc As Long
    Dim HeightSrc As Long
    Dim rc As RECT
    GetWindowRect WndHandle, rc
    WidthSrc = rc.Right - rc.Left
    HeightSrc = rc.Bottom - rc.Top
    
    'Get Window  device context
    hDCSrc = GetWindowDC(WndHandle)
    
    'create a memory device context
    hDCMemory = CreateCompatibleDC(hDCSrc)
    
    'create a bitmap compatible with window hdc
    hBmp = CreateCompatibleBitmap(hDCSrc, WidthSrc, HeightSrc)
    'hBmp is YourBmp, hBmpPrev is 0
    
    Debug.Print "hbmp after createcompatible: " & hBmp
    
    'copy newly created bitmap into memory device context
    hBmpPrev = SelectObject(hDCMemory, hBmp)
    'hBmp is YourBmp, hBmpPrev is OtherBmp

    'copy window window hdc to memory hdc
    Call BitBlt(hDCMemory, 0, 0, WidthSrc, HeightSrc, hDCSrc, 0, 0, vbSrcCopy)
      
    'Get Bmp from memory Dc
    hBmp = SelectObject(hDCMemory, hBmpPrev)
    'hBmp is YourBmp, hBmpPrev is OtherBmp

    'copy window window hdc to memory hdc
    Call BitBlt(hDCMemory, 0, 0, WidthSrc, HeightSrc, hDCSrc, 0, 0, vbSrcCopy)
      
    Set c = New c32bppDIB

    Dim b As Boolean
    b = c.LoadPicture_ByHandle(hBmp)
    Debug.Assert b
    
End Sub
Public Sub CleanUp()

    DeleteObject hBmp
    
    DeleteDC hDCMemory
    
    Set c = Nothing
    
    m_bCleant = True
    
    Debug.Print "after cleanup: " & hBmp

End Sub
Private Sub Class_Terminate()

    If Not m_bCleant Then
        Me.CleanUp
    End If

End Sub

And that's how I call it:

Private Sub btnTest_Click()

    Dim l&
    For l = 1 To 50

        Dim cSS As clsScreenshot
        Set cSS = New clsScreenshot
        cSS.TakeScreenshot
        cSS.c.SaveToFile_PNG "d:\blatest1.png", False
        cSS.CleanUp
        Set cSS = Nothing
        
    Next l
    
End Sub

[LaVolpe]

Well, when you run your project, ensure all IDE MDI child windows are closed and the immediate window also, if it applies. If they aren't, exit the running project, close those windows and try again. Only at that time, take the GDI handle count. If not, those windows are probably the reason for the 2 you noted.

Now back to your revised class...
Why do you have this line in there twice? Probably a mistake

Call BitBlt(hDCMemory, 0, 0, WidthSrc, HeightSrc, hDCSrc, 0, 0, vbSrcCopy)

Still recommend calling ReleaseDC before you exit your TakeScreenshot routine. I also think you should move your bitmap deletion to that routine instead of the cleanup, unless there is a reason to keep a large bitmap laying around, not being used? You probably should check whether hBmp and hDCMemory are non-zero before attempting to delete them. Also, reset their values to zero afterwards, so you know you deleted them.

I don't recall whether the LoadPicture_ByHandle routine keeps the bitmap or copies it? If it keeps it, then leave the bitmap deletion where it is if that class doesn't delete it when the class terminates. If the class deletes the bitmap, then there is no reason for you to be deleting it.

[tmighty2]

Man, thank you!

I totally forget adding

    ReleaseDC WndHandle, hDCSrc

That resolved the leak.