Memory leak
Memory leak
Memory leak
Memory leak
Memory leak
Memory leak Memory leak Memory leak Memory leak Memory leak Memory leak Memory leak Memory leak
Memory leak Memory leak
Memory leak
Go Back  Xtreme Visual Basic Talk > > > Memory leak


Reply
 
Thread Tools Display Modes
  #1  
Old 03-21-2011, 08:24 PM
tbasham tbasham is offline
Newcomer
 
Join Date: Mar 2009
Posts: 7
Default Memory leak


I am fairly new to visual basic and have written this program to detect a change in a selected pixel on the screen. When the pixel changes, a sound plays every x seconds.

It works fine but after a few hours of operation a possible memory leak causes it to either crash the program or the PC. I have watched the process and I can see 4k added to it every 4 seconds or so.

Any ideas on where this memory leak is coming from?

Here is the code:
Code:
Public Class Main
    Public pointx As Long
    Public pointy As Long
    Public areaselected As Integer
    Public TimeToPause, SecondTimer, changefound As Integer
    Public AlertTone As String


    Public LocalMousePosition As Point
    Public color1 As Integer
    Public color2 As Integer
    Public Declare Function GetDC Lib "user32" Alias "GetDC" (ByVal hwnd As IntPtr) As Integer
    Public Declare Function GetPixel Lib "gdi32" (ByVal hdc As Integer, ByVal x As Integer, ByVal y As Integer) As Integer

    'Private WithEvents timer2 As New Windows.Forms.Timer
    Private Declare Function GetAsyncKeyState Lib "user32" (ByVal vKey As Long) As Integer


    Private Sub Button2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs)
        Form2.Show()
    End Sub



    Public Function PixelCol(ByVal x As Integer, ByVal y As Integer) As Integer
        Dim DC As Integer = GetDC(0)
        Return GetPixel(DC, x, y)
    End Function



    Sub PlayBackgroundSoundFile()


        If ComboBox1.SelectedIndex = 0 Then
            My.Computer.Audio.Play(My.Resources.sound1, AudioPlayMode.Background)
        ElseIf ComboBox1.SelectedIndex = 1 Then
            My.Computer.Audio.Play(My.Resources.sound2, AudioPlayMode.Background)
        ElseIf ComboBox1.SelectedIndex = 2 Then
            My.Computer.Audio.Play(My.Resources.sound3, AudioPlayMode.Background)
        ElseIf ComboBox1.SelectedIndex = 3 Then
            My.Computer.Audio.Play(My.Resources.sound4, AudioPlayMode.Background)
        ElseIf ComboBox1.SelectedIndex = 4 Then
            My.Computer.Audio.Play(My.Resources.sound5, AudioPlayMode.Background)
        ElseIf ComboBox1.SelectedIndex = 5 Then
            My.Computer.Audio.Play(My.Resources.voice1, AudioPlayMode.Background)
        ElseIf ComboBox1.SelectedIndex = 6 Then
            My.Computer.Audio.Play(My.Resources.voice2, AudioPlayMode.Background)
        End If

    End Sub


    Private Sub PictureBox2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles PictureBox2.Click

        PlayBackgroundSoundFile()

    End Sub



    Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
        Form2.Show()
    End Sub

    Private Sub ComboBox2_SelectedIndexChanged(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles ComboBox2.SelectedIndexChanged
        If ComboBox2.SelectedIndex = 0 Then
            TimeToPause = 30
        ElseIf ComboBox2.SelectedIndex = 1 Then
            TimeToPause = 60
        ElseIf ComboBox2.SelectedIndex = 2 Then
            TimeToPause = 90
        ElseIf ComboBox2.SelectedIndex = 3 Then
            TimeToPause = 120
        ElseIf ComboBox2.SelectedIndex = 4 Then
            TimeToPause = 150
        End If
    End Sub

    

    Private Sub Timer2_Tick_1(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Timer2.Tick
        If GetAsyncKeyState(67) Then

            pointx = MousePosition.X
            pointy = MousePosition.Y



            color1 = PixelCol(pointx, pointy)
            areaselected = 1

            Me.WindowState = FormWindowState.Minimized
        End If


        '----------- main loop

        If areaselected = 1 Then
            If changefound = 0 Then
                color2 = PixelCol(pointx, pointy)

                If color1 <> color2 Then

                    PlayBackgroundSoundFile()
                    '
                    'Threading.Thread.Sleep(TimeToPause) ' wait x seconds before checking again
                    changefound = 1


                End If
            

            ElseIf changefound = 1 Then
                SecondTimer = SecondTimer + 1
                If SecondTimer = TimeToPause Then
                    changefound = 0
                    SecondTimer = 0
                End If
            End If
        End If

        ' ------------ end main loop

    End Sub
End Class

Last edited by Flyguy; 03-22-2011 at 02:36 AM. Reason: Added [code] [/code] tags
Reply With Quote
  #2  
Old 03-21-2011, 09:27 PM
PlausiblyDamp's Avatar
PlausiblyDampMemory leak PlausiblyDamp is offline
Ultimate Contributor

Forum Leader
* Expert *
 
Join Date: Nov 2003
Location: Newport, Wales
Posts: 2,058
Default

Not sure if this is the only problem but you are calling GetDC repeatedly in the Timer's tick event - if you look at the documentation for GetDC (http://msdn.microsoft.com/en-us/library/aa921543.aspx) then it explicitly states that you are expected to also call ReleaseDC when you are finished with it.
__________________
Intellectuals solve problems; geniuses prevent them.
-- Albert Einstein

Posting Guidelines Forum Rules Use the code tags
Reply With Quote
  #3  
Old 03-22-2011, 08:13 AM
AtmaWeapon's Avatar
AtmaWeaponMemory leak AtmaWeapon is offline
Fabulous Florist

Forum Leader
* Guru *
 
Join Date: Feb 2004
Location: Austin, TX
Posts: 9,500
Default

Personally, I'd promote the DC to a class-level variable rather than creating one every time the timer ticks. You are supposed to release a resource when you're done with it, but in this case you're not done with it until the application exits.
__________________
.NET Resources
My FAQ threads | Tutor's Corner | Code Library
I would bet money 2/3 of .NET questions are already answered in one of these three places.
Reply With Quote
  #4  
Old 03-22-2011, 06:03 PM
tbasham tbasham is offline
Newcomer
 
Join Date: Mar 2009
Posts: 7
Default Memory leak

If I do a ReleaseDC, what would the syntax be for my program above? I'm a bit confused by it.

temp = ReleaseDC(???, ???) ?

Thanks in advance!
Reply With Quote
  #5  
Old 03-22-2011, 08:25 PM
Qua's Avatar
QuaMemory leak Qua is offline
Impetuous & volatile

* Expert *
 
Join Date: Apr 2005
Posts: 2,177
Default

Did you look at the documentation for ReleaseDC? In accordance with the documentation you should pass the release method the window handle as well as the DC handle. In return you will get an integer; 1 indicated successful release.
__________________
Reading is the foundation for all knowledge - Unknown.
Reply With Quote
  #6  
Old 03-22-2011, 09:34 PM
tbasham tbasham is offline
Newcomer
 
Join Date: Mar 2009
Posts: 7
Default

Yes, I read the documentation as well as other documentation on it. The problem is that I got the program working by a lot of trial and error and what I am confused about is what is the window handle and DC handle as it relates to my program.

I believe I have the line of code right, but I just can't figure out what to put between the parenthesis.

Thanks!
Reply With Quote
  #7  
Old 03-23-2011, 08:38 AM
Rockoon's Avatar
Rockoon Rockoon is offline
Joseph Koss

* Guru *
 
Join Date: Aug 2003
Location: Unfashionable End
Posts: 3,615
Default

The window handle is the handle you gave to GetDC() when you obtained the DC, and the DC handle is the value that was returned by your call to GetDC()
Reply With Quote
  #8  
Old 03-23-2011, 09:05 AM
tbasham tbasham is offline
Newcomer
 
Join Date: Mar 2009
Posts: 7
Default

Ok, so if I'm understanding correctly it looks like it would be

temp = ReleaseDC(DC, color2)

I will give that a try tonight.

Thanks for the help.
Reply With Quote
  #9  
Old 03-23-2011, 05:02 PM
Rockoon's Avatar
Rockoon Rockoon is offline
Joseph Koss

* Guru *
 
Join Date: Aug 2003
Location: Unfashionable End
Posts: 3,615
Default

The first parameter is the window handle, the second is the dc, and in the source posted above the window handle was '0', not 'color2'

(note: a window handle of '0' is a special-case constant that means 'the entire desktop')

Is it too much to ask that you at least look at the documentation (that was even linked to in this thread) for ReleaseDC?
Reply With Quote
  #10  
Old 03-23-2011, 08:54 PM
tbasham tbasham is offline
Newcomer
 
Join Date: Mar 2009
Posts: 7
Default

Is it too much to ask that you at least look at the documentation (that was even linked to in this thread) for ReleaseDC?

If you read the statements from my previous posts you would see that:

a) "Yes, I read the documentation as well as other documentation on it." and "...am confused by it"
b) "I am fairly new to visual basic..."

I thought this was a place to as questions and get help, not be insulted.

But I do appreciate your other suggestions and will see if that works.

Last edited by tbasham; 03-23-2011 at 09:35 PM.
Reply With Quote
  #11  
Old 03-24-2011, 09:35 AM
AtmaWeapon's Avatar
AtmaWeaponMemory leak AtmaWeapon is offline
Fabulous Florist

Forum Leader
* Guru *
 
Join Date: Feb 2004
Location: Austin, TX
Posts: 9,500
Default

I assure you that Rockoon didn't mean to offend. You have no idea how many people come here, ask a question, and completely ignore any responses. Usually they're looking for a quick copy/paste answer and at this forum we generally find that detestable, particularly for simple issues.

Here's why Rockoon was so frustrated.

The documentation gives these signatures for GetDC() and ReleaseDC():
Code:
HDC GetDC(HWND hWnd);
int ReleaseDC(HWND hWnd, HDC hDC);
GetDC() creates a device context for the hWnd you pass in. It returns an HDC. ReleaseDC() releases a device context that was created with GetDC(); it takes an HWND and an HDC. Maybe it's just experience talking, but to us it feels like a no-brainer to assume that when you're given a "create" and a "release" that use the same types for everything, you'd pass the thing you created to the thing you released.

There is a bit of a campfire argument you got yourself into; let's talk about it.

I've been programming in some form for 15 years now. I've been through BASIC, Pascal, C, C++, and .NET, and nearly a dozen other languages. I started out with trial and error programming, but it always ended in failure. My first real completed program was a clone of DrugWars in TI-83+ BASIC. Once it had more than about 4 screens of code, I realized it was getting too big to understand. This started the tradition of The Notebook; I always have a notebook that is where I jot everything about programs. I filled probably 25 pages with notes about each label in the program (no subroutines!), each variable (I had A-Z and theta, 27 whole variables!), and overall architecture of how the program would work. When I identified I needed a new feature, I had to consult the notebook to figure out where the code might fit and whether I had any free variables.

This is intended as anecdotal evidence for a hypothesis: trial and error programming will get you through simple scenarios, but it never leads to professional code. I love to sit down and hack at stuff without up-front planning, but the more complicated the things I try the quicker I find I get stuck because my own design is inadequate. This is when I pull out The Notebook. I cannot stress how much better programming works when you do some planning up-front.

The problem: this has led me and many other experienced developers to view programming by trial and error as something only done by novice programmers. If we're not careful, we fall in the trap of forgetting that we had to learn the dangers of trial and error the hard way.

Here's a counterpoint to assuming trial and error means a wrong practice. From time to time when I'm following the design I wrote in The Notebook, I start hitting snags. Sometimes it turns out my design wasn't such a good idea and it's not working very well when I try and express it in code. Sometimes it's that I wrote "this class does X" when I don't really understand what X is. When I hit this snag, I close The Notebook and start trying to hack out solutions. I quit caring about organization or any other good practices: my goal is to do the most natural thing that makes it work. It makes a mess, and the end results are always ugly, but I don't quit until X is working. When it's working, I go back to The Notebook and start writing again: now I know a way to do X and I can try to design a better way. Odds are many who think trial and error isn't a good practice do this and don't realize it; they call it "prototyping" but that's just putting a bow on it. It's got a place in projects. Ideally, you throw away this code when you're done because you weren't thinking about writing good code when writing it. Like G.I. Joe said, knowing is half the battle.

It shouldn't be our job to belittle people who do it, it should be our job to teach them how to think better. Often what happens is one of us gets a little testy, and another jumps in and teaches the lesson. To be honest, I'm one of the quickest to be mean on this forum and I've had to let the experts posting in this thread clean up the messes. I'll return the favor.

Here's how you *should* have approached the problem. You didn't approach it this way because perhaps you haven't ever had anyone show you how to think this way. Don't see this as an insult; see it as instructive. There is a vast chasm between "unaware", "educated", and "idiot"; the only way to go from "unaware" to one of the others is to be told the way.

You got stuck when you couldn't understand what ReleaseDC() did. So you started plugging in random variables in your program until you found one that worked. This is officially what The Pragmatic Programmer labeled Programming by Accident; it's a specific form of trial and error that's worse than more structured versions. Program code is a series of instructions for the computer; it is imperative that you know what every line of code you write does when you write it. Sometimes you don't know what a function does, so you just try it out. That's fine, but you should at least have an idea of what you expect it to do.

So right here you should have asked yourself, "Am I using the right parameters for ReleaseDC()? If I wrote ReleaseDC(), how would it look?" Let's go through that second question. We know GetDC() takes an HWND that represents the window and it returns an HDC that represents the device context. So if I wanted to write ReleaseDC(), I'd expect at minimum that it take that HDC. So already we know that our code should look like this:
Code:
Dim dc As HDC = GetDC(0)

ReleaseDC(dc)
But there's that pesky second parameter to ReleaseDC(). What the heck does the HWND parameter do?
Quote:
Handle to the window whose device context is to be released.
OK, it's a handle to the window that owns the DC. We had to pass an HWND to GetDC(), I wonder if that's related?
Quote:
Handle to the window whose device context is to be retrieved. If this value is NULL, GetDC retrieves the device context for the entire screen.
Ahh, the HWND that is passed to GetDC() is the owner of the device context. (That bit about NULL isn't super relevant, though note that passing 0 is the same as passing NULL in API.)

So now I have a new idea of what it should look like. Here's the process of using GetDC() and ReleaseDC()
  1. Find the HWND for the window for which you'd like a DC. (In this case 0 for the whole screen.)
  2. Pass the window's HWND to GetDC(); this returns the DC.
  3. Do some stuff with the DC.
  4. Call ReleaseDC(), giving it the HWND from step 1 and the DC from step 2.
In code:
Code:
Dim hWnd As Integer = 0
Dim dc As HDC = GetDC(hWnd)
' do some stuff
ReleaseDC(hWnd, dc)
Now the final step: ask yourself what's different between this theoretical code and your own code. Knowing those differences helps you catch yourself making the same error later. Your code probably looks something like this:
Code:
Dim DC As Integer = GetDC(0)
Dim colorValue As Integer = GetPixel(DC, x, y)
ReleaseDC(DC, colorValue)
Return colorValue
Ahh, now we see the problem. ReleaseDC() takes the HWND and HDC in that order. Your code gives the HDC and a color. Now we understand the changes that need to be made:
Code:
Dim DC As Integer = GetDC(0)
Dim colorValue As Integer = GetPixel(DC, x, y)
ReleaseDC(0, DC)
Return colorValue
Now let's say you did all that work and you were still stuck on the last part. That's where you ask yourself more questions. "ReleaseDC() takes an HWND for a window and an HDC for a device context. I am passing an HDC and an integer that represents a color." That tells you you're doing something wrong. It's OK if you don't understand the right thing to do, but making a post that says "I know these are the wrong parameters, but for some reason these are the only things that make sense to me" does a lot to make us old guys slower to meanness. We've all been there and we still get stuck on really dumb things from time to time. But we're a lot happier to answer, "I know ReleaseDC() takes a window handle and device context handle but for some reason I don't feel like I have that information" than we are to answer "I picked two random variables and can't figure out why it doesn't work."

In the end, I feel that what separates a novice programmer from a professional is whether "I don't know what this line of code does" seems scary. The novice responds with "What's the worst that could happen?" and that's exactly the problem. If you don't know the worst possible side effects, then you can't be upset if it deletes your wedding photos. I'm comfortable with, "I don't know if I'm using this function properly" but only if it's coupled with "But the worst side effect is harmless." By analogy: executing code you don't understand is like giving a loaded gun to a toddler; you're hoping the child doesn't manage to pull the trigger. It's not so dangerous to hand an unloaded gun to a child (trial and error with limited knowledge) but a trained gun owner would still recognize it as irresponsible because guns are not toys.

So don't be that guy. If you read this post and learn a little bit about how to learn more about *why* things aren't working, you're one step closer to professional. If you ignore this post in favor of copy/pasting the code, you are starting to swim the river between "unware" and "idiot". That hurts everyone involved and in that case you will quickly find this forum of limited utility.
__________________
.NET Resources
My FAQ threads | Tutor's Corner | Code Library
I would bet money 2/3 of .NET questions are already answered in one of these three places.
Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off

Forum Jump

Advertisement:





Free Publications
The ASP.NET 2.0 Anthology
101 Essential Tips, Tricks & Hacks - Free 156 Page Preview. Learn the most practical features and best approaches for ASP.NET.
subscribe
Programmers Heaven C# School Book -Free 338 Page eBook
The Programmers Heaven C# School book covers the .NET framework and the C# language.
subscribe
Build Your Own ASP.NET 3.5 Web Site Using C# & VB, 3rd Edition - Free 219 Page Preview!
This comprehensive step-by-step guide will help get your database-driven ASP.NET web site up and running in no time..
subscribe
Memory leak
Memory leak
Memory leak Memory leak
Memory leak
Memory leak
Memory leak Memory leak Memory leak Memory leak Memory leak Memory leak Memory leak
Memory leak
Memory leak
 
Memory leak
Memory leak
 
-->