How to fix "terminate called without an active exception" error in this code?

Question

First of all, I am a student so the code might not be an efficient or well-written one. Sorry for that. But I could use some help if it doesn't bother you.

Following is my main program and musical_chairs function that I use in my main program.

void musical_chairs(IntQueueHW6 &my_queue, unsigned int idx,unsigned int remaining_player_num, vector<int> &players)
{

    this_thread::sleep_until(chrono::system_clock::now() + chrono::seconds(2));
    mtx.lock();
    if(!(my_queue.isFull()))
    {
        my_queue.enqueue(idx);// my_queue is a queue with size of total_player_num-1
        cout << "Player " << idx << " captured a chair at ";
        current_time();
        cout << "." << endl;
    }
    else
    //**************** I believe this block causing the error *******************
        not_capture(idx);
        for(int k = 0; k < remaining_player_num; k++)
        {
            if(players[k] == idx)
            {
                players.erase(players.begin()+k);
                break;
            }
        }
    }
    mtx.unlock();
//*************** I believe this block causing the error **********************
}

int main()
{
    unsigned int total_player_num, total_round_num, remaining_player_num;
    cout << "Welcome to Musical Chairs game!" << endl << "Enter the number of players in the game: " << endl;
    cin >> total_player_num;
    remaining_player_num = total_player_num;
    total_round_num = total_player_num - 1;
    cout << "Game Start!" << endl << endl;

    vector<int> players(total_player_num); // creating a vector consists of players IDs
    for(int i=0; i < total_player_num; i++)
    {
        players[i] = i;
    }


    while(remaining_player_num != 1) // until only one player left in te game
    {
        IntQueueHW6 my_queue(total_round_num); // creating a queue with size total_round_num for chair slots
        vector<thread> threads(total_player_num); // create a thread vector for all players

        cout << "Time is now ";
        current_time();
        cout << endl;
        int players_length = players.size();

        for(int i = 0; i < players_length; i++)
        {
            int idx = players[i];
            threads[idx] = thread(&musical_chairs, ref(my_queue), idx, remaining_player_num, ref(players));
        }
        
        for(int i = 0; i < players_length; i++) //joining to threads
        {
            int idx = players[i];
            if(threads[idx].joinable())
            {
                threads[idx].join();
            }
        }
         
        display_remaining_players(players); // changing variables accordingly
        remaining_player_num --;
        total_round_num -- ;
        my_queue.clear();
    }

    unsigned int winner_id = players[0];
    winner_func(winner_id); // this function displays the winner

    return 0;
}

I wrote a program that simulates "musical chairs" game with using multi-threading. When I run the main program I get "terminate called without an active exception" error. Due to my little research on the internet it may be because of threads going out of scope but if it is I do not know how to fix it. Can anyone explain how to solve it? If the error occurs from a different thing can you please explain it and can you help me to fix this code? Thanks in advance.

Answer 1

The std::terminate is called because a thread is terminating without being joined.

The losing thread in the musical_chairs function removes itself from the vector of players, but in the meantime the main thread has continued executing, looping over the threads calling join. The lack of synchronization here will cause this to fail in random ways depending on the vagaries of thread scheduling.

Given the code structure, this will take a re-think to fix. The following is the simplest implementation I could come up with. It uses std::atomic instead of std::mutex which is much more efficient.

A couple of quick notes:

• I chose a std::set instead of a std::vector for the players because the player ids are unique and it is much easier and more efficient to remove an element from a set than a vector.
• To make the game fairer, I chose the middle thread as the loser in each round -- otherwise, the last thread was losing too often.
• The lambda capture for the thread creation is subtle. The pid must be captured by value so that each thread will have its own player id, but the queue, chairs, and loser need to be captured by reference because they are all shared across threads.

Sample Code

#include <chrono>
#include <iostream>
#include <mutex>
#include <queue>
#include <set>
#include <thread>
#include <vector>

using std::cin, std::cout, std::endl;
using namespace std::chrono_literals;

using PlayerId = int;
using Queue = std::queue<PlayerId>;
using Players = std::set<PlayerId>;

int main(int argc, const char *argv[]) {
    int initial_number_players{};
    cin >> initial_number_players;

    Players players;
    for (auto i = 0; i < initial_number_players; ++i)
        players.insert(i);

    while (players.size() > 1) {
        Queue queue;
        std::vector<std::thread> threads;

        std::atomic<int> chairs{}, loser{};
        for (auto pid : players) {
            threads.emplace_back([&,pid]() {
                std::this_thread::sleep_for(250ms);
                auto cid = chairs.fetch_add(1);
                if (cid == players.size() / 2)
                    loser = pid;
            });
        }
        for (auto& th : threads)
            if (th.joinable())
                th.join();

        cout << "Player " << loser << " did not capture a chair" << endl;
        players.erase(loser);
    }

    cout << "Player " << *players.begin() << " won" << endl;

    return 0;
}

Output

Player 8 did not capture a chair
Player 0 did not capture a chair
Player 1 did not capture a chair
Player 2 did not capture a chair
Player 6 did not capture a chair
Player 3 did not capture a chair
Player 5 did not capture a chair
Player 4 did not capture a chair
Player 9 did not capture a chair
Player 7 won