DevilWAH
DevilWAH

Reputation: 2633

c# memory leak in loop

public void DoPing(object state)
{
    string host = state as string;
    m_lastPingResult = false;
    while (!m_pingThreadShouldStop.WaitOne(250))
    {
        Ping p = new Ping();
        try
        {
            PingReply reply = p.Send(host, 3000);
            if (reply.Status == IPStatus.Success)
            {
                m_lastPingResult = true;
            }
            else
            {
                m_lastPingResult = false;
            }
        }
        catch
        {

        }
        numping = numping + 1;
    }
}

Any idea why this code gives me a memory leak? I can see it's this code as changing the wait value to smaller or larger values increases the rate of the memory usage. Does any one have any idea how to resolve it? or how to see what part of the code is causing it?

Upvotes: 1

Views: 4161

Answers (5)

Starlyn Diaz
Starlyn Diaz

Reputation: 1

This can be used from a static Class:

public static bool testNet(string pHost, int pTimeout)
{
    Ping p = new Ping();

    bool isNetOkay = false;

    int netTries = 0;

    do
    {
        PingReply reply = p.Send(pHost, pTimeout);

        if (reply.Status == IPStatus.Success)
        {
            isNetOkay = true;
            break;
        }

        netTries++;

    } while (netTries < 4);

    //Void memory leak

    p.Dispose();

    return isNetOkay;
}

Upvotes: 0

Antarr Byrd
Antarr Byrd

Reputation: 26061

using(var p = new Ping())
{
    try
    {
        var reply = p.Send(host, 3000);

        if (reply.Status == IPStatus.Success)
           _lastPingResult = true;
        else  
           _lastPingResult = false; 
    }
    catch(Exception e)
    {
       //...
    }
}

Upvotes: 1

Diego
Diego

Reputation: 18349

Add a finally statement to your try-catch, like this:

catch() {}
finally
{
     Ping.Dispose();
}

Upvotes: 2

Yaur
Yaur

Reputation: 7452

Garbage collection only happens when there is memory pressure, thus just seeing your memory usage go up doesn't mean there is a memory leak and in this code I don't see how there could be a legitimate leak. You can add

GC.Collect();
GC.WaitForPendingFinalizers();

to double check but shouldn't leave that in production.

Edit: someone in comments pointed out that Ping is Disposable. not calling dispose can cause leaks that will eventually get cleaned up but may take a long time and cause non memory related problems.

Upvotes: 2

sethcall
sethcall

Reputation: 2897

In some garbage collected languages, there is a limitation that the object isn't collected if the method that created it still hasn't exited.

I believe .net works this way in debug mode. Quoting from this article; note the bolded statement.

http://www.simple-talk.com/dotnet/.net-framework/understanding-garbage-collection-in-.net/

A local variable in a method that is currently running is considered to be a GC root. The objects referenced by these variables can always be accessed immediately by the method they are declared in, and so they must be kept around. The lifetime of these roots can depend on the way the program was built. In debug builds, a local variable lasts for as long as the method is on the stack. In release builds, the JIT is able to look at the program structure to work out the last point within the execution that a variable can be used by the method and will discard it when it is no longer required. This strategy isn’t always used and can be turned off, for example, by running the program in a debugger.

Upvotes: 5

Related Questions