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?

4
  • 1
    If you're able to start over, make a Worker Service? Commented Mar 19, 2020 at 15:13
  • 2
    Oh dear. Google "async void considered harmful" to get ahead. Commented Mar 19, 2020 at 16:27
  • I can make my thread more efficient by using async/await <-- In what way the thread becomes more efficient? Commented Mar 19, 2020 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. Commented Mar 20, 2020 at 22:36

2 Answers 2

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.

Sign up to request clarification or add additional context in comments.

2 Comments

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.
Excellent find. Will edit. - Done.
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.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.