0

I've written this program to automatically gather the IP Addresses that allow me to connect to port 8888.

It works fine until the final loop. Whereby my for loop finishes. But my timer keeps going and outputting:

10.10.10.150 - No
10.10.10.150 - No
10.10.10.150 - No
10.10.10.150 - No

This is my code, after the for loop I try stop the timer, but it doesn't stop.

protected void PingPython(){

    for (int i = 50; i <= 150; i++){

        // Try Connect to Python
        try{
            ip = "10.10.10."+i.ToString();

            // Set timer to break Client connection
            tConnectTimeout = new System.Timers.Timer(100);
            tConnectTimeout.Elapsed += new System.Timers.ElapsedEventHandler(tConnectTimeout_Elapsed);
            tConnectTimeout.Start();

            // Connect to Client
            cli = new TcpClient();
            cli.Connect(ip, 8888);

            // If it connects, stop the thread
            tConnectTimeout.Stop();
            tConnectTimeout.Dispose();
            Console.WriteLine(ip + " - Yes");
            ipAddresses.Add(ip);
            cli.Close();

        } catch (ObjectDisposedException ex) {
        } catch (SocketException ex) {
            tConnectTimeout.Stop();
            tConnectTimeout.Dispose();
            Console.WriteLine(ip + " - No");
        }

    }

    tConnectTimeout.Stop();
    btnStart.Sensitive = true;
    foreach(string ipa in ipAddresses){
        cbAddresses.AppendText(ipa);
    }
    cbAddresses.Sensitive = true;
}

public void tConnectTimeout_Elapsed(object sender, System.Timers.ElapsedEventArgs e){
    //Close the socket
    cli.Close();
    Console.WriteLine(ip + " - No");

    //Stop and dispose timer
    tConnectTimeout.Stop();
    tConnectTimeout.Dispose();
}
Jon Egerton
  • 40,401
  • 11
  • 97
  • 129
Dobz
  • 1,213
  • 1
  • 14
  • 36
  • Please add an answer that shows the answer to your problem instead of re-editing the question. Thanks – Jon Egerton Jul 15 '14 at 14:06
  • 1
    Are you really just trying to implement a connection timeout? http://stackoverflow.com/a/17118710/56778 for a cleaner way to do it. – Jim Mischel Jul 15 '14 at 18:05

2 Answers2

3

try using a using statement to create the timer (and dispose of it no matter the excetion)

using (var timer = new system.timer)
{




}

instead of using an exception handler to do it. Also why the empty exception handler around a huge scope? that is bad you want that removed or scoped to minimum possible..

what is happening is that something is going bang causing your for loop to exit in effect before the timer can be stopped so it keeps firing the event.

in response to jon edgerton the solution shuold be

protected void PingPython(){

for (int i = 50; i <= 150; i++){

    // Try Connect to Python
    try{
        ip = "10.10.10."+i.ToString();

        // Set timer to break Client connection
        tConnectTimeout = new System.Timers.Timer(100);
        tConnectTimeout.Elapsed += new System.Timers.ElapsedEventHandler(tConnectTimeout_Elapsed);
        tConnectTimeout.Start();

        // Connect to Client
        cli = new TcpClient();
        cli.Connect(ip, 8888);


        Console.WriteLine(ip + " - Yes");
        ipAddresses.Add(ip);
        cli.Close();

    } catch (ObjectDisposedException ex) {
    } catch (SocketException ex) {
        Console.WriteLine(ip + " - No");
    }
    finally
    {
         tConnectTimeout.Stop();
         tConnectTimeout.Dispose();
    }

}

tConnectTimeout.Stop();
btnStart.Sensitive = true;
foreach(string ipa in ipAddresses){
    cbAddresses.AppendText(ipa);
}
cbAddresses.Sensitive = true;

}

or better yet

for (int i = 50; i <= 150; i++)
{

    // Try Connect to Python
    try
    {
        ip = "10.10.10."+i.ToString();

        // Set timer to break Client connection
        using( tConnectTimeout = new System.Timers.Timer(100))
        {
            tConnectTimeout.Elapsed += new System.Timers.ElapsedEventHandler(tConnectTimeout_Elapsed);
            tConnectTimeout.Start();

            // Connect to Client
             using (cli = new TcpClient())
             {
                 cli.Connect(ip, 8888);

                 tConnectTimeout.Stop()

                 Console.WriteLine(ip + " - Yes");
                 ipAddresses.Add(ip);
                 cli.Close();
             }
        }
    } 
    catch (ObjectDisposedException ex) 
    {
    } 
    catch (SocketException ex) 
    {
        Console.WriteLine(ip + " - No");
    }
}
John Nicholas
  • 4,778
  • 4
  • 31
  • 50
  • It turns out it was the Empty Exception Handler giving me the problem. The final IP was giving that error, which in turn didn't close the thread. I've added the Thread.Close() to that Exception and now it works. – Dobz Jul 15 '14 at 13:53
  • yeah sorry should of seen that immediatley you should use usings in dispose patterns – John Nicholas Jul 15 '14 at 13:53
  • The reason the Empty Handler was there in the first place was because the Application would immediately close if it wasn't. – Dobz Jul 15 '14 at 13:54
  • @RussellHickey: @JohnNicholas is right though - you're much better to use the `using` pattern to deal with disposables – Jon Egerton Jul 15 '14 at 14:09
  • @JonEgerton @JohnNicholas I haven't seen a `using` before. I'd need to look more into it before implementing it. – Dobz Jul 15 '14 at 14:33
  • what i have above is the syntax ... you are doing stuff in a catch ... that should be in a finally ... the using effectivley writes the dispose int eh finally for you. – John Nicholas Jul 15 '14 at 14:43
  • 1
    you really should reduce the scope of that try to th epart that can fail though. You makeyour life harder if you dont. Things can get v hard to track down. – John Nicholas Jul 15 '14 at 14:57
0

From the OPs edits, the answer was to change the catch block to the following:

    } catch (ObjectDisposedException ex) {
        //HERE WAS THE PROBLEM. Added these two lines and now working.
        tConnectTimeout.Stop();
        tConnectTimeout.Dispose();
    } catch (SocketException ex) {
        tConnectTimeout.Stop();
        tConnectTimeout.Dispose();
        Console.WriteLine(ip + " - No");
    }

(see here)

However I wouldn't recommend this fix over correct use of using to manage the timer.

Community
  • 1
  • 1
Jon Egerton
  • 40,401
  • 11
  • 97
  • 129