Is there a memory leak in my code?
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!
Code:
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:
Code:
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
Re: Is there a memory leak in my code?
You should put the code
hBmp = SelectObject(hDCMemory, hBmpPrev)
DeleteObject hBmp
Call DeleteDC(hDCMemory)
At the end of you procedure
Re: Is there a memory leak in my code?
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!
Re: Is there a memory leak in my code?
Quote:
Originally Posted by
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!
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.
Re: Is there a memory leak in my code?
I am setting the class instance to "nothing" when I'm done.
Like that:
Code:
Dim c As New clsScreenshot
c.TakeScreenshot
Set c = Nothing
Is that sufficient to actually terminate the class?
Re: Is there a memory leak in my code?
@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:
Code:
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
Re: Is there a memory leak in my code?
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
Re: Is there a memory leak in my code?
Quote:
Originally Posted by
tmighty2
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:
Code:
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
Re: Is there a memory leak in my code?
@LaVolpe
Thank you, but am I not destroying hBmp here?
-->
Code:
hBmp = SelectObject(hDCMemory, hBmpPrev)
DeleteObject hBmp
I have changed my initial post to include just everything, I think that becomes important now.
Re: Is there a memory leak in my code?
Quote:
Originally Posted by
tmighty2
@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.
Re: Is there a memory leak in my code?
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:
Code:
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:
Code:
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
Re: Is there a memory leak in my code?
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.
Re: Is there a memory leak in my code?
Man, thank you!
I totally forget adding
Code:
ReleaseDC WndHandle, hDCSrc
That resolved the leak.
