3
\$\begingroup\$

I was working on the last days with threads and I didn't want to use ugly methods like Abort(), etc.

So I did a simple class which does use one of the most performant and better alternative to Abort(). The class is named as CThread.cs which would stand for CleanThread.cs.

Class code:

using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

public class CThread
{
    public List<CancellationTokenSource> CreatedThreads = new List<CancellationTokenSource>();

    private void MyFunction(Action callback, bool inLoop, int currentIndex)
    {
        if (inLoop)
        {
            while (!CreatedThreads[currentIndex].IsCancellationRequested)
            {
                callback();
            }
        }
        else { callback(); }
    }

    private int getLastIndex()
    {
        int index = 0;
        foreach (CancellationTokenSource item in CreatedThreads)
        {
            ++index;
        }

        return (index - 1);
    }

    public new void Start(Action callback, bool inLoop)
    {
        CancellationTokenSource tokenSource = new CancellationTokenSource();

        CreatedThreads.Add(tokenSource);
        int currentIndex = getLastIndex();
        Task task = Task.Run(() => MyFunction(callback, inLoop, currentIndex)); // use Task.Factory.StartNew for .net 4.0
    }

    public new void Stop(int threadIndex)
    {
        CreatedThreads[threadIndex].Cancel();
        Console.WriteLine("Thread has been stopped successfully! Press any key to continue...");
        Console.ReadKey();
    }
}

and finally, how I use it:

using System;

namespace CThreadExample
{
    internal class Program
    {
        private static void printFunction()
        {
            Console.WriteLine("Hello world!");
            System.Threading.Thread.Sleep(120);
        }

        private static void Main(string[] args)
        {
            CThread myThread = new CThread();
            myThread.Start(printFunction, true);

            // when the user presses any key the thread will be stopped
            Console.ReadKey();
            myThread.Stop(0); // stops the thread 0
        }
    }
}

Well, as you can see I'm not able right now to start threads using multiple parameters, but I didn't need to pass parameter to my thread.

Since I want to make everything clean, I accept any suggestion to improve this, also small suggestions for the code to make me able to pass one, two or more parameters and for cross-threading too (the Control.CheckForIllegalCrossThread = false; alternative way - so using delegates with Invoke() or BeginInvoke()).

\$\endgroup\$
6
\$\begingroup\$

This doesn't actually work as intended. You're using the CancellationTokenSource as a bool to decide whether to re-invoke the callback. That's not stopping a thread; that is not re-invoking the callback when it returns. The only reason why it appears to work for you is:

  • Your test callback is very short. Try inserting a long-running callback.
  • You print "Thread has been stopped successfully! Press any key to continue..." right after calling Cancel(). That statement is false; the only thing that has happened at that point is the CancellationTokenSource has been cancelled.

Other non-functional issues:

  • The CreatedThreads list doesn't hold threads, it holds CancellationTokenSources. Name the list appropriately.
  • Why is CreatedThreads public? It seems like something you wouldn't want the class to expose. Make it private.
  • This method is very strange:
private int getLastIndex()
{
    int index = 0;
    foreach (CancellationTokenSource item in CreatedThreads)
    {
        ++index;
    }

    return (index - 1);
}

Why does it not just return CreatedThreads.Count - 1 rather than iterating through the whole list? At that point there's no need for a method for it; Start can just do int currentIndex = CurrentThreads.Count - 1;

  • It is very awkward to have the Stop method take an index of the thread to stop, especially when an index was not supplied to or returned from Start. How is the caller of the method to know what index their thread is?

Finally I assume the only reason why you are calling Console methods in the CThread class is because you're using it for testing. Otherwise you definitely should not be doing that.

\$\endgroup\$
  • \$\begingroup\$ Thank you. Regarding to getLastIndex() function, I figured it out it was trash, because I tried to use just the Last() function (as for Lists this function is available but it doesn't return an int, so I had some problems), not to use its Count property.Regarding to misinterpreting CancellationTokenSource with threads, OK, I will take care (I wanted just to do a "simulation", I know actually they aren't thread). Regarding to that is not re-invoking the callback when it returns, it actually is, just tested. Why wouldn't it work with long-running callback? What do you mean with long-run? \$\endgroup\$ – YesICanCSharp May 8 '16 at 13:46
  • \$\begingroup\$ This is what's happening. You're calling Cancel() on the CancellationTokenSource. That does nothing apart from change that object's state; it has nothing to do with the callback you're running. The only way it is effecting the callback is inside a loop, by making you stop invoking the callback (while (not cancelled) { invoke callback }). // ... continued... \$\endgroup\$ – 404 May 8 '16 at 13:58
  • \$\begingroup\$ ..continued... But if the callback takes 1 minute to execute, and you call Stop() right after Start(), the callback will not stop executing; after a minute it will return, and because the CTS was cancelled, it won't be invoked again; but you didn't stop the thread or the task being executed, you only stopped the loop from invoking the callback again. I assume you want the running callback to stop when you call Stop, not continue running for another 10 minutes or however long it takes to complete, otherwise I don't see the point of this class. \$\endgroup\$ – 404 May 8 '16 at 14:02
  • \$\begingroup\$ Re: long running, change your printFunction method to this: for (int i = 0; i < int.MaxValue; i++) { Console.WriteLine("Hello world!"); Thread.Sleep(20); } and see what happens when you try to stop it. \$\endgroup\$ – 404 May 8 '16 at 14:05
  • \$\begingroup\$ you are right. What do you suggest me to do to stop waiting for that routine to finish? \$\endgroup\$ – YesICanCSharp May 8 '16 at 14:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.