Using a Mutex to Limit the Number of Threads Running at a Time to 2

Question

I have a program that pushes 10 threads into a vector, each of which is supposed to print out a character 5 times before finishing ('A' for the first thread, 'B' for the second, etc). I'm able to get them to either run all at once (using detach()) or have them run one at a time (using join()). Now I want to use a Mutex to limit the number of threads allowed to print at a time to 2. I've been able to declare the mutex and put the lock in place but I'm unsure of how to apply a limit like this. Anyone have any ideas on how to proceed?

deque<int> q ;
mutex print_mutex ;
mutex queue_mutex ;
condition_variable queue_cond ;

void begin(int num) {
    unique_lock<mutex> ul {queue_mutex};
    q.emplace_back(num);
    queue_cond.wait(ul,[num]{
        return q.front() == num; });
    q.pop_front();
    cout << num << " leaves begin " << endl ;
}

void end ( int num ) {
    lock_guard<mutex>lg{queue_mutex};
    queue_cond.notify_all();
    cout << num << " has ended " << endl ;
}

void run(int num, char ch) {
    begin(num);
    for (int i = 0; i < 5; ++i) {
        {
            lock_guard<mutex> lg { print_mutex };
            cout << ch << endl << flush ;
        }
        sleep_for(milliseconds(250));
    }
    end(num);
}

int main() {
    vector<thread>threads {};
    for (int i = 0; i < 10; ++i) {
        threads.push_back(thread{run,i,static_cast<char>(65+i)});
        threads.at(i).join();
    }
}

You need an integer somewhere to count the number of running threads. — David Schwartz, May 08 '17 at 21:45
Using `detach()` and `join()` to control sequencing is wrong. The `for` loop in `main` simply serializes the threads; that's pointless. Use a loop to create **all** of the threads, followed by a separate loop that joins **all** of the threads. Then figure out how you want the threads to interact. Don't use `detach()`; it's definitely not what you need. — Pete Becker, May 08 '17 at 22:04
@PeteBecker Great catch, that was my problem! Excellent explanation. I was trying to add in a better lock condition but nothing was changing until I separated the join into another loop. If you'd like to go ahead and post that as an answer I would be happy to accept it. — gmooney8, May 08 '17 at 22:37

score 3 · Accepted Answer · answered May 08 '17 at 22:45

You have already set up a FIFO for your threads with the global deque<int> q. So let's use that.

Currently, you're trying to restrict execution until the current thread is at the front. Although there's a bug, because begin will immediately pop that thread from the deque. Better to remove the value when you call end. Here's that change, first:

void end(int num)
{
    {
        lock_guard<mutex>lg{queue_mutex};
        cout << num << " has ended " << endl ;
        q.erase(find(q.begin(), q.end(), num));
    }
    queue_cond.notify_all();
}

This uses std::find from <algorithm> to remove the specific value. You could use pop_front, but we're about to change that logic so this is more generic. Also notice you don't need to lock the condition variable when you notify.

So, it's not much of a stretch to extend the logic in begin to be in the first two places. Here:

void begin(int num)
{
    unique_lock<mutex> ul {queue_mutex};
    q.emplace_back(num);
    queue_cond.wait(ul,[num]{
        auto end = q.begin() + std::min(2, static_cast<int>(q.size()));
        return find(q.begin(), end, num) != end;
        });
    cout << num << " leaves begin " << endl ;
}

You can change that 2 to anything you want, allowing up to that many threads to pass. At some point, you would probably abandon this approach and use something simpler like a single counter variable, then rely on the thread scheduler to manage which thread is woken, rather than force them into your FIFO. That way you can switch to using notify_one to wake a single thread and reduce switching overhead.

Anyway, the last thing to do is remove the join from your thread generation loop. The concurrency is now managed by begin and end. So you would do this:

for (int i = 0; i < 10; ++i) {
    threads.push_back( thread{run, i, 'A'+i} );
}
for (auto & t : threads) t.join();

Great explanation, paddy! I wound up finding a different solution (using the counter that you mentioned) but your explanation makes much better sense of it. Thank you! — gmooney8, May 08 '17 at 23:52
One thing I forgot to mention is that your begin/end code is not exception-safe. If your thread terminates due to an exception, then `end` will never be called. You might consider wrapping the begin/end calls inside an object so that RAII will take care of it. — paddy, May 09 '17 at 01:20

Using a Mutex to Limit the Number of Threads Running at a Time to 2

1 Answers1