3

I have a program I am writing that will run a variety of tasks. I have set up what I have called a "Task Queue" in which I will continually grab the next task to process (if there is one) and start a new thread to handle that task. However, I want to limit the amount of threads that can spawn at one time for apparent reasons. I created a variable to keep up with the max threads to spawn and one for the current thread count. I was thinking of using a lock to try and accurately keep up with the current thread count. Here is my general idea.

public class Program {
  private static int mintThreadCount;
  private static int mintMaxThreadCount = 10;
  private static object mobjLock = new object();
  static void Main(string[] args) {
     mintThreadCount = 0;
     int i = 100;
     while(i > 0) {
        StartNewThread();
        i--;
     }
     Console.Read();
  }

  private static void StartNewThread() {
     lock(mobjLock) {
        if(mintThreadCount < mintMaxThreadCount) {
           Thread newThread = new Thread(StartTask);
           newThread.Start(mintThreadCount);
           mintThreadCount++;
        }
        else {
           Console.WriteLine("Max Thread Count Reached.");
        }
     }
  }

  private static void StartTask(object iCurrentThreadCount) {
     int id = new Random().Next(0, 1000000);
     Console.WriteLine("New Thread with id of: " + id.ToString() + " Started. Current Thread count: " + ((int)iCurrentThreadCount).ToString());
     Thread.Sleep(new Random().Next(0, 3000));
     lock(mobjLock) {
        Console.WriteLine("Ending thread with id of: " + id.ToString() + " now.");
        mintThreadCount--;
        Console.WriteLine("Thread space release by id of: " + id.ToString() + " . Thread count now at: " + mintThreadCount);
     }
  }
}

Since I am locking in two places to access the same variable (increment when starting the new thread and decrement when ending it) is there a chance that the thread waiting on the lock to decrement could get hung up and never end? Thereby reaching max thread count and never being able to start another one? Any alternate suggestions to my method?

mac
  • 485
  • 1
  • 6
  • 29
  • I'd be inclined to read about [Thread Pooling](https://msdn.microsoft.com/en-us/library/h4732ks0.aspx), as it takes care of a lot of your concerns. – Charles Mager Apr 22 '15 at 16:09
  • 3
    Have a look at [SemaphoreSlim](https://msdn.microsoft.com/en-us/library/system.threading.semaphoreslim(v=vs.110).aspx) – oleksii Apr 22 '15 at 16:11
  • Yeah, what @oleksii said: use `SemaphoreSlim`. See a simple example at http://stackoverflow.com/a/19594643/56778 – Jim Mischel Apr 22 '15 at 16:26
  • I should also mention that in Main() it will be while(true) and not limited to 100. I want it to continually run. Therefore the main thread should never be blocked. So I see Thread Pooling is useful but I'm not sure it fits exactly with what I'm trying to accomplish. I started to go with Semaphore but I wanted a way to ensure that only a certain amount of threads could exists at one time and not just restrict the number of threads in a block of code at one time. This way I'm not spawning an endless amount of threads then running a certain amount of tasks at a time but spawning limited threads. – mac Apr 22 '15 at 17:31

1 Answers1

3

Easiest question first… :)

…is there a chance that the thread waiting on the lock to decrement could get hung up and never end?

No, not in the code you posted. None of the code holds a lock while waiting for the count to change, or anything like that. You only ever take the lock, then either modify the count or emit a message, and immediately release the lock. So no thread will hold the lock indefinitely, nor are there nested locks (which could lead to deadlock if done incorrectly).

Now, that said: from the code you posted and your question, it's not entirely clear what the intent here is. The code as written will indeed limit the number of threads created. But once that limit is reached (and it will do so quickly), the main loop will just spin, reporting "Max Thread Count Reached.".

Indeed, with a total loop count of 100, I think it's possible that the entire loop could finish before the first thread even gets to run, depending on what else is tying up CPU cores on your system. If some threads do get to run and it happens that some of them get very low durations to sleep, there's a chance that you might sneak in a few more threads later. But most of the iterations of the loop will see the thread count at the maximum, report the limit has been reached and continue with the next iteration of the loop.

You write in the comments (something you should really put in the question itself, if you think it's relevant) that "the main thread should never be blocked". Of course, the question there is, what is the main thread doing when not blocked? How will the main thread know if and when to try to schedule a new thread?

These are important details, if you want a really useful answer.

Note that you've been offered the suggestion of using a semaphore (specifically, SemaphoreSlim). This could be a good idea, but note that that class is typically used to coordinate multiple threads all competing for the same resource. For it to be useful, you'd actually have more than 10 threads, with the semaphore ensuring that only 10 get to run at a given time.

In your case, it seems to me that you are actually asking how to avoid creating the extra thread in the first place. I.e. you want the main loop to check the count and just not create a thread at all if the maximum count is reached. In that case, one possible solution might be to just use the Monitor class directly:

private static void StartNewThread() {
    lock(mobjLock) {
        while (mintThreadCount >= mintMaxThreadCount) {
           Console.WriteLine("Max Thread Count Reached.");
           Monitor.Wait(mobjLock);
        }

        Thread newThread = new Thread(StartTask);
        newThread.Start(mintThreadCount);
        mintThreadCount++;
        }
    }
}

The above will cause the StartNewThread() method to wait until the count is below the maximum, and then will always create a new thread.

Of course, each thread needs to signal that it's updated the count, so that the above loop can be released from the wait and check the count:

private readonly Random _rnd = new Random();

private static void StartTask(object iCurrentThreadCount) {
    int id = _rnd.Next(0, 1000000);
    Console.WriteLine("New Thread with id of: " + id.ToString() + " Started. Current Thread count: " + ((int)iCurrentThreadCount).ToString());
    Thread.Sleep(_rnd.Next(0, 3000));
    lock(mobjLock) {
        Console.WriteLine("Ending thread with id of: " + id.ToString() + " now.");
        mintThreadCount--;
        Console.WriteLine("Thread space release by id of: " + id.ToString() + " . Thread count now at: " + mintThreadCount);
        Monitor.Pulse(mobjLock);
    }
}

The problem with the above is that it will block the main loop. Which if I understood correctly, you don't want.

(Note: you have a common-but-serious bug in your code, in that you create a new Random object each time you want a random number. To use the Random class correctly, you must create just one instance and reuse it as you want new random numbers. I've adjusted the code example above to fix that problem).

One of the other problems, both with the above, and with your original version, is that each new task is assigned a brand new thread. Threads are expensive to create and even to simply exist, which is why thread pools exist. Depending on what your actual scenario is, it's possible that you should just be using e.g. the Parallel, ParallelEnumerable, or Task to manage your tasks.

But if you really want to do this all explicitly, one option is to simply start up ten threads, and have them retrieve data to operate on from a BlockingCollection<T>. Since you start exactly ten threads, you know you'll never have more than that running. When there is enough work for all ten threads to be busy, they will be. Otherwise, the queue will be empty and some or all will be waiting for new data to show in the queue. Idle, but not using any CPU resources.

For example:

private BlockingCollection<int> _queue = new BlockingCollection<int>();

private static void StartThreads() {
    for (int i = 0; i < mintMaxThreadCount; i++) {
        new Thread(StartTask).Start();
    }
}

private static void StartTask() {
    // NOTE: a random number can't be a reliable "identification", as two or
    // more threads could theoretically get the same "id".
    int id = new Random().Next(0, 1000000);
    Console.WriteLine("New Thread with id of: " + id.ToString() + " Started.");
    foreach (int i in _queue) {
        Thread.Sleep(i);
    }
    Thread.Sleep(new Random().Next(0, 3000));
}

You'd call StartThreads() just once somewhere, rather than calling your other StartNewThread() method multiple times. Presumably, before the while (true) loop you mentioned.

Then as the need to process some task, you just add data to the queue, e.g.:

_queue.Add(_rnd.Next(0, 3000));

When you want the threads to all exit (e.g. after your main loop exits, however that happens):

_queue.CompleteAdding();

That will cause each of the foreach loops in progress to end, letting each thread exit.

Of course, the T type parameter for BlockingCollection<T> can be anything. Presumably, it will be whatever in your case actually represents a "task". I used int, only because that was effectively your "task" in your example (i.e. the number of milliseconds the thread should sleep).

Then your main thread can just do whatever it normally does, calling the Add() method to dispatch new work to your consumer threads as needed.

Again, without more details I can't really comment on whether this approach would be better than using one of the built-in task-running mechanisms in .NET. But it should work well, given what you've explained so far.

Peter Duniho
  • 68,759
  • 7
  • 102
  • 136
  • Thanks for the detailed response. The Random was just for testing and looking at the threads in the console (wasn't paying attention ;)). The main thread reporting max count is fine. I just needed a way to continually look for tasks to process (thousands of different daily tasks for my company) but without spawning endless threads. After reading your response I guess my question really should have been: Are the thread requests for a lock queued or is there a chance one will never be able to obtain the lock to decrement after completing it's task? I do like your approach though. – mac Apr 23 '15 at 13:10
  • _"Are the thread requests for a lock queued"_ -- yes, the `Monitor` class (which is the basis for the `lock` statement) uses a wait queue and a ready queue. Threads are serviced FIFO. – Peter Duniho Apr 23 '15 at 17:24