C# thread called multiple times but runs once

Question

Basically I have a form with a button, when the button is pressed it creates an instance of a class that runs a Thread. When the thread is done, it automatically calls Thread.Abort().

The code I currently have comes down to this:

Button:

private void Buttonclick(object sender, EventArgs e)
{
     MyClass c = new MyClass()
     c.Do_your_thing();
}

Class:

public class MyClass
{
    Thread t;

    public void Do_your_thing()
    {
         t = new Thread(Running_code);
         t.Start();
    }

    private void Running_code()
    {
         //Perform code here
         t.Abort();
    }
}

When I click the button once, everything works. But when I press the button again, nothing happens.

When I don't use t.Abort() everything works. But not using t.Abort() will cause memory leaks and the program won't close properly (the thread is never closed, therefor the process will stay alive).

Can anyone explain me what is going on? And how can I fix it?

EDIT: as per request I am posting some actual code

public class MyClass
{
    public void Test()
    {
        t = new Thread(() =>
            {
                wb.DocumentCompleted += get_part;
                wb.Navigate("http://www.google.com");
                Application.Run();
            });

        t.SetApartmentState(ApartmentState.STA);
        t.Start();
    }

    public void get_part(object sender, WebBrowserDocumentCompletedEventArgs e)
    {
        var br = sender as WebBrowser;
        string url = e.Url.ToString();

        //Here is some code that compares the url to surten predefined url. When there is a match, it should run some code and then go to a new url

        if(url == string_final_url)
        {
            //Finally at the url I want, open it in a new Internet Explorer Window
            Process proc = Process.Start("IExplore.exe", url);           
        }
    }
}

This is a fraction of a little webscraper program. It navigates to a webpage that needs some login info. When I reached the page I actually want to be, he should open it in a new Internet Explorer.

When I call this code and close the form, it's still visible in the process tree. And when I click the button multiple times, the memory used keeps getting higher, which I suspected to be some sort of memory leak.

Answer 1

Firstly, don't use Thread.Abort(), ever. See Is this thread.abort() normal and safe? for more details on why.

There are many warnings all over the net about using Thread.Abort(). I would recommend avoiding it unless it's really needed, which in this case, I don't think it is. You'd be better off just implementing a one-shot timer, with maybe a half-second timeout, and resetting it on each keystroke. This way your expensive operation would only occur after a half-second or more (or whatever length you choose) of user inactivity.

Instead of using abort you can use the Join() Method. This method blocks the calling thread until a thread terminates.

An example of it use is

Thread t1 = new Thread(() => 
{ 
    Thread.Sleep(4000);
    Console.WriteLine("t1 is ending.");
});
t1.Start();

Thread t2 = new Thread(() => 
{ 
    Thread.Sleep(1000);
    Console.WriteLine("t2 is ending.");
});
t2.Start();

t1.Join();
Console.WriteLine("t1.Join() returned.");

t2.Join();
Console.WriteLine("t2.Join() returned.");

I hope this helps.

---

Edit. To address your comments; The call to Join() is what de-allocates the thread. You don't have to do anything else. Just make sure that the threads clean up any resources they might be using before they exit.

That said, I would urge you to look into using the thread pool or the Task Parallel Library (TPL) rather than explicitly managing threads. They're easier to use, and handle this kind of thing much more smoothly.

Answer 2

Are you able to utilize .net 4 + if so you can use the TPL which would greatly simpify this as

public class MyClass
    {
        public void Do_your_thing()
        {
            // for async execution
            Task.Factory.StartNew(Running_code);

            // for synchronous execution
            // CAUTION !! If invoked from UI thread this will freeze the GUI until Running_code is returned.
            //Task.Factory.StartNew(Running_code).Wait(); 
        }

        private void Running_code()
        {
           Thread.Sleep( 2000 );
           Debug.WriteLine( "Something was done" );
        }
    }

Moreover if the Running_Code method was doing something IO bound the TPL can utilize the IO completion ports and the operation may be completely threadless.

EDIT:

Have a look at this SO thread. WebBrowser Control in a new thread.

Apparently webbrowser control does not play well with non UI threads.