0

Let's say I have a MyThread class in my Windows C# app like this:

public class MyThread
{
   Thread TheThread;

   public MyThread()
   {
      TheThread = new Thread(MyFunc);
   }

   public void StartIfNecessary()
   {
      if (!TheThread.IsAlive)
         TheThread.Start();
   }

   private void MyFunc()
   {
      for (;;)
      {
          if (ThereIsStuffToDo)
             DoSomeStuff();
      }
   }
}

That works fine. But now I realize I can make my thread more efficient by using async/await:

public class MyThread
{
   Thread TheThread;

   public MyThread()
   {
      TheThread = new Thread(MyFunc);
   }

   public void StartIfNecessary()
   {
      if (!TheThread.IsAlive)
         TheThread.Start();
   }

   private async void MyFunc()
   {
      for (;;)
      {
         DoSomeStuff();
         await MoreStuffIsReady();
      }
   }
}

What I see now, is that the second time I call StartIfNecessary(), TheThread.IsAlive is false (and ThreadState is Stopped BTW) so it calls TheThread.Start() which then throws the ThreadStateException "Thread is running or terminated; it cannot restart". But I can see that DoMoreStuff() is still getting called, so the function is in fact still executing.

I suspect what is happening, is that when my thread hits the "await", the thread I created is stopped, and when the await on MoreStuffIsReady() completes, a thread from the thread pool is assigned to execute DoSomeStuff(). So it is technically true that the thread I created has been stopped, but the function I created that thread to process is still running.

So how can I tell if "MyFunc" is still active?

I can think of 3 ways to solve this:

1) Add a "bool IsRunning" which is set to true right before calling TheThread.Start(), and MyFunc() sets to false when it completes. This is simple, but requires me to wrap everything in a try/catch/finally which isn't awful but I was hoping there was a way to have the operating system or framework help me out here just in case "MyFunc" dies in some way I wasn't expecting.

2) Find some new function somewhere in System.Threading that will give me the information I need.

3) Rethink the whole thing - since my thread only sticks around for a few milliseconds, is there a way to accomplish this same functionality without creating a thread at all (outside of the thread pool)? Start "MyFunc" as a Task somehow?

Best practices in this case?

Betty Crokker
  • 3,001
  • 6
  • 34
  • 68
  • 1
    If you're able to start over, make a [Worker Service](https://medium.com/@nickfane/introduction-to-worker-services-in-net-core-3-0-4bb3fc631225)? – gunr2171 Mar 19 '20 at 15:13
  • 2
    Oh dear. Google "async void considered harmful" to get ahead. – Hans Passant Mar 19 '20 at 16:27
  • *I can make my thread more efficient by using async/await* <-- In what way the thread becomes more efficient? – Theodor Zoulias Mar 19 '20 at 19:05
  • The `MyFunc()` method returns as soon as it hits the `await` statement. That's the end of that thread. See marked duplicate. See also the thousands of related, and in many cases practically identical, questions on Stack Overflow that help explain how async/await works and how you can apply those techniques to your own code. – Peter Duniho Mar 20 '20 at 22:36

2 Answers2

1

Sticking with a Plain Old Thread and using BlockingCollection to avoid a tight loop:

class MyThread
{
    private Thread worker = new Thread(MyFunc);
    private BlockingCollection<Action> stuff = new BlockingCollection<Action>();

    public MyThread()
    {
        worker.Start();
    }

    void MyFunc()
    {
        foreach (var todo in stuff.GetConsumingEnumerable())
        {
           try
           {
               todo();
           }
           catch(Exception ex)
           {
              // Something went wrong in todo()
           }
        }
        stuff.Dispose(); // should be disposed!
    }

    public void Shutdown()
    {
         stuff.CompleteAdding(); // No more adding, but will continue to serve until empty.
    }

    public void Add( Action stuffTodo )
    {
          stuff.Add(stuffTodo); // Will throw after Shutdown is called
    }
}

BlockingCollection also shows examples with Task if you prefer to go down that road.

Fildor
  • 14,510
  • 4
  • 35
  • 67
  • 1
    Usually you would use `GetConsumingEnumerable` instead of doing it the low-level way where you need to manually check if the collection was completed or has any elements. I.e. `foreach (var todo in stuff.GetConsumingEnumerable())` is much simpler than the while-loop, the Take call and the catching of an InvalidOperationException. – ckuri Mar 19 '20 at 16:58
  • Excellent find. Will edit. - Done. – Fildor Mar 19 '20 at 17:00
1

Rethink the whole thing

This is definitely the best option. Get rid of the thread completely.

It seems like you have a "consumer" kind of scenario, and you need a consumer with a buffer of data items to work on.

One option is to use ActionBlock<T> from TPL Dataflow:

public class NeedsADifferentName
{
  ActionBlock<MyDataType> _block;

  public NeedsADifferentName() => _block = new ActionBlock<MyDataType>(MyFunc);

  public void QueueData(MyDataType data) => _block.Post(data);

  private void MyFunc(MyDataType data)
  {
    DoSomeStuff(data);
  }
}

Alternatively, you can build your own pipeline using something like Channels.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810