0

I want to open a thread to do the things it needs to do until a new command is given by the user. Then this thread should either close or receive a new command.

I have seen many posts that sending a variable to a running thread is hard, that is why I decided to kill the thread and start it again with the new variable.

I used the following post: https://stackoverflow.com/a/1327377 but without success. When I start the thread again (after it has done abort()) it gives me an exception: System.Threading.ThreadStateException.

private static Thread t = new Thread(Threading);
private static bool _running = false;

static void Main(string[] args)
{
    [get arg]
    if (CanRedo(arg))
    {
        if (t.IsAlive)
        {
            _running = false;
            t.Interrupt();
            if (t.Join(2000)) // with a '!' like in the post, abort() would not be called
            {
                t.Abort();
            }
        }
        _running = true;
        t.Start(arg); // gives System.Threading.ThreadStateException
    }
}



private static void Threading(object obj)
{
    _stopped = false;
    string arg = obj.ToString();
    while(_running)
    {
        if (bot._isDone)
        {
            ExecuteInstruction(arg);
        }
    }   
}

What am I doing wrong?

Enigmativity
  • 113,464
  • 11
  • 89
  • 172
wes
  • 379
  • 3
  • 15
  • 1
    Once a Thread object completed, either by finishing normally or by shooting it in the head like that, it is no longer usable to execute code. You must create a new instance, use the `new` keyword. – Hans Passant Apr 29 '19 at 14:18
  • Calling `Thread.Abort()` is dangerous and should be avoided at all cost. It can leave the run-time in an invalid state and cause corruption for all the remaining threads. You should only ever abort a thread when you want to crash out of your app. – Enigmativity May 01 '19 at 10:22
  • It would be super awesome to have code that actually compiles and runs - even if it then fails. Your code is all over the place and it's hard to understand what you're trying to do, so concrete code answers are impossible. – Enigmativity May 01 '19 at 10:27

2 Answers2

2

I'm going to guess that you don't literally mean to abort the thread and start that same thread again. That's because if we start a thread to do some work we don't care which thread it is. If you cancel one thing and start something else, you probably don't care if it's the same thread or a different one. (In fact it's probably better if you don't care. If you need precise control over which thread is doing what then something has gotten complicated.) You can't "abort" a thread and restart it anyway.

Regarding Thread.Abort:

The Thread.Abort method should be used with caution. Particularly when you call it to abort a thread other than the current thread, you do not know what code has executed or failed to execute when the ThreadAbortException is thrown, nor can you be certain of the state of your application or any application and user state that it is responsible for preserving. For example, calling Thread.Abort may prevent static constructors from executing or prevent the release of unmanaged resources.

It's like firing an employee by teleporting them out of the building without warning. What if they were in the middle of a phone call or carrying a stack of papers? That might be okay in an emergency, but it wouldn't be a normal way to operate. It would be better to let the employee know that they need to wrap up what they're doing immediately. Put down what you're carrying. Tell the customer that you can't finish entering their order and they'll need to call back.

You're describing an expected behavior, so it would be better to cancel the thread in an orderly way.

That's where we might use a CancellationToken. In effect you're passing an object to the thread and telling it to check it from time to time to see if it should cancel what it's doing.

So you could start your thread like this:

class Program
{
    static void Main(string[] args)
    {
        using (var cts = new CancellationTokenSource())
        {
            ThreadPool.QueueUserWorkItem(DoSomethingOnAnotherThread, cts.Token);

            // This is just for demonstration. It allows the other thread to run for a little while
            // before it gets canceled.
            Thread.Sleep(5000);
            cts.Cancel();
        }
    }

    private static void DoSomethingOnAnotherThread(object obj)
    {
        var cancellationToken = (CancellationToken) obj;

        // This thread does its thing. Once in a while it does this:

        if (cancellationToken.IsCancellationRequested)
        {
            return;
        }

        // Keep doing what it's doing.
    }
}

Whatever the method is that's running in your separate thread, it's going to check IsCancellationRequested from time to time. If it's right in the middle of doing something it can stop. If it has unmanaged resources it can dispose them. But the important thing is that you can cancel what it does in a predictable way that leaves your application in a known state.

CancellationToken is one way to do this. In other really simple scenarios where the whole thing is happening inside one class you could also use a boolean field or property that acts as a flag to tell the thread if it needs to stop. The separate thread checks it to see if cancellation has been requested.

But using the CancellationToken makes it more manageable if you want to refactor and now the method executing on another thread is a in separate class. When you use a known pattern it makes it easier for the next person to understand what's going on.

Here's some documentation.

Scott Hannen
  • 27,588
  • 3
  • 45
  • 62
0

What about doing it this way:

      private static Task t = null;
      private static CancellationTokenSource cts = null;

      static void Main(string[] args)
      {
         [get arg]
         if (CanRedo(out var arg))
         {
            if (t != null)
            {
               cts.Cancel();
               t.Wait();
            }
            // Set up a new task and matching cancellation token
            cts = new CancellationTokenSource();
            t = Task.Run(() => liveTask(arg, cts.Token)); 
         }
      }

      private static void liveTask(object obj, CancellationToken ct)
      {
         string arg = obj.ToString();
         while(!ct.IsCancellationRequested)
         {
            if (bot._isDone)
            {
               ExecuteInstruction(arg);
            }
         }   
      }

Tasks are cancellable, and I can see nothing in your thread that requires the same physical thread to be re-used.

Steve Todd
  • 1,250
  • 6
  • 13