views:

55

answers:

2

I have asked this question before - but I have spent some time thinking about it and have implemented a working version.

Overview
1) Threads are being created to perform a certain task.
2) Only one thread can perform the task at a time.
3) Each thread performs the exact same task. (Does a bunch of checks and validations on a system)
3) The threads are being created faster than the task can be performed. (I have no control over the thread creation)

Result is that overtime I get a backlog of threads to perform the task.

What I have implemented goes as follows
1) Thread checks to see how many active threads there are.
2) If there are 0 threads it is marked to PerformTask and it starts the task
3) If there is 1 thread it is marked to PerformTak and it blocks
4) If there is more than 1 thread the thread is not marked to PerformTasks and just dies

The idea is that if there is a thread waiting to perform the task already I just kill the thread.

Here is the code that I came up with

              bool tvPerformTask = false;

              ivNumberOfProcessesSemaphore.WaitOne();
              if (ivNumberOfProcessesWaiting == 0 ||
                  ivNumberOfProcessesWaiting == 1)
              {
                ivNumberOfProcessesWaiting++;
                tvPerformTask = true;
              }
              ivNumberOfProcessesSemaphore.Release();

              if (tvPerformTask)
              {
                //Here we perform the work
                ivProcessSemaphore.WaitOne();
                //Thread save

                ivProcessSemaphore.Release();


                ivNumberOfProcessesSemaphore.WaitOne();
                ivNumberOfProcessesWaiting--;
                ivNumberOfProcessesSemaphore.Release();
              }
              else
              {
                //we just let the thread die
              }

The problem that I have is not that it doesn't work it is just that I do not find the code elegant specifically I am not very happy that I need 2 semaphores an integer and a local flag to control it all. If there a way to implement this or pattern that would make the code simpler.

A: 

Consider using a ThreadPool instead of trying to managing the creation and destruction of individual threads on your own.

Donnie
I don't have control over the thread creation. I am implementing a class to perform a task which is called from another part of the system.
John Soer
...I was thinking along these same lines - coupled with some logic to ensure you don't end up creating too many threads as the workload is higher than the system can handle.
Will A
This doesn't seem to address the question...
Adam Robinson
+1  A: 

How about this?

private readonly _lock = new object();
private readonly _semaphore = new Semaphore(2, 2);

private void DoWork()
{
    if (_semaphore.WaitOne(0))
    {
        try
        {
            lock (_lock)
            {
                // ...
            }
        }
        finally
        {
            _semaphore.Release();
        }
    }
}
ChaosPandion
+1 after the edit. Good solution!
Adam Robinson
This would cause all threads to perform the tasks. If there is a backlog of threads I just want to discard the threads. Is there a way to check on the Semaphore if it would block a thread. If the semaphore blocks I could just discard the thread.
John Soer
@John: No, it wouldn't. If `_semaphore.WaitOne(0)` returns `false`, that means that there's one performing and one waiting, so it simply exits.
Adam Robinson
@Adam - Nice call. @John - In my code the semaphore will only allow 2 threads to enter the if statement and only 1 thread will be able to take the lock. All the other threads will not block.
ChaosPandion
Sweet that is awesome.
John Soer