1

Im trying to make a mini threads game, should be a manager thread that create amount of thread as the user want, and for every round (the user choose the amount of rounds) the threads need to random number, and after all the threads finished to random number the managerGame thread should print the thread that random the greatest number and add point to this thread. At the end of the game the ManagerGame thread should print who is the winner. Now i wrote the next code:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <pthread.h>

struct Player{
    int isFinished;
    int id;
    int number;
};

struct GameManager{
    struct Player *Players;
    int *WinnersTable;
    int Rounds;
};

struct GameDetails{
    int PlayersCount;
    int RoundsCount;
};

struct GameManager game_manager;
pthread_mutex_t lock;
pthread_cond_t cond;
int IsGameActive;

void* ManagerGameFunc(void* gameDetails);
void* PlayerFunc(void* playerDetails);
void SetWinnerToRound(int roundNumber, int playersCount);
void StartNewRound(int playersCount);
void printWinnerToGame(int playersCount);
int IsRoundEnded(int playersCount);

int main(){
    srand(time(NULL));
    pthread_t ManagerThread;
    pthread_mutex_init(&lock, NULL);
    pthread_cond_init(&cond, NULL);
    struct GameDetails gameDetails;
    printf("----GAME----\n");
    printf("Enter amount of players\n");
    fflush(stdin);
    scanf("%d\n", &gameDetails.PlayersCount);
    printf("Enter amount of rounds\n");
    fflush(stdin);
    scanf("%d\n", &gameDetails.RoundsCount);
    printf("----GAME STARTED----\n");
    pthread_create(&ManagerThread, NULL, ManagerGameFunc, &gameDetails);
    pthread_join(ManagerThread, NULL);
    printf("Game finished\n");
    pthread_mutex_destroy(&lock);
    pthread_cond_destroy(&cond);
}

void* ManagerGameFunc(void* gameDetails){
    struct GameDetails game_details = *((struct GameDetails *)gameDetails);
    pthread_t *threads = (pthread_t *)malloc(game_details.PlayersCount * sizeof(pthread_t));
    game_manager.Players = (struct Player *)malloc(game_details.PlayersCount * sizeof(struct Player));
    game_manager.WinnersTable = (int *)malloc(game_details.PlayersCount * sizeof(int));
    game_manager.Rounds = game_details.RoundsCount;
    IsGameActive = 1;
    for(int i = 0; i < game_details.PlayersCount; i++){
        game_manager.Players[i].isFinished = 0;
        game_manager.Players[i].id = i;
        pthread_create(&threads[i], NULL, PlayerFunc, &game_manager.Players[i]);
    }
    for(int i = 0; i < game_manager.Rounds; i++){
        pthread_mutex_lock(&lock);
        while(!IsRoundEnded(game_details.PlayersCount)){
            pthread_cond_wait(&cond, &lock);
        }
        SetWinnerToRound(i + 1, game_details.PlayersCount);
        StartNewRound(game_details.PlayersCount);
        pthread_cond_signal(&cond);
        pthread_mutex_unlock(&lock);
    }
    IsGameActive = 0;
    printf("\nGame end ! The winner of the game is ...\n");
    printWinnerToGame(game_details.PlayersCount);
}

void* PlayerFunc(void* playerDetails){
    struct Player player = *((struct Player *)playerDetails);
    while(IsGameActive){
        pthread_mutex_lock(&lock);
        while(player.isFinished){
            pthread_cond_wait(&cond, &lock);
        }
        player.number = (rand() % (100 - 1 + 1)) + 1;
        printf("%d", player.number);
        player.isFinished = 1;
        pthread_cond_signal(&cond);
        pthread_mutex_unlock(&lock);
    }
}

void SetWinnerToRound(int roundNumber, int playersCount){
    int max = game_manager.Players[0].number;
    int id = 0;
    for(int i = 1; i < playersCount; i++){
        if(game_manager.Players[i].number > max){
            max = game_manager.Players[i].number;
            id = i;
        }
    }
    printf("--Round %d: winner is player number %d--\n", roundNumber, id + 1);
    game_manager.WinnersTable[id]++;
}

void StartNewRound(int playersCount){
    for(int i = 1; i < playersCount; i++){
        game_manager.Players[i].isFinished = 0;
    }
}

void printWinnerToGame(int playersCount){
    int max = game_manager.Players[0].number;
    int id = 0;
    for(int i = 1; i < playersCount; i++){
        if(game_manager.Players[i].number > max){
            max = game_manager.Players[i].number;
            id = i;
        }
    }
    printf("Player number %d is the winner of the game !!\n", id + 1);
}

int IsRoundEnded(int playersCount){
    for(int i = 0; i < playersCount; i++)
        if(!game_manager.Players[i].isFinished)
            return 0;
    return 1;
}

But the problem is that when i run it the output is the image is in the link and the terminal is stuck like it show in the image. I think that maybe the threads is in deadlock mode, isnt it? Thank you for your help!

UPDATE

So I heard to @n.m. and to @pilcrow, and in PlayerFunc i changed the Player object to pointer to the argument cause before the change the function made a copy of the argument and every time i tried to change something in the object it didnt change in the PlayerThread. In addition i changed the pthread_cond_signal to pthread_cond_broadcast and now it work. Thank you all!

Avi C
  • 13
  • 4
  • Re: `fflush(stdin);` - [Using fflush(stdin)](https://stackoverflow.com/q/2979209/2505965) – Oka Feb 07 '23 at 22:54
  • Re: `scanf("%d\n", ...)` - [What is the effect of trailing white space in a scanf() format string?](https://stackoverflow.com/q/19499060/2505965) – Oka Feb 07 '23 at 23:01
  • If every other line is empty, your program is more difficult to read. – n. m. could be an AI Feb 08 '23 at 06:08
  • The synchronisation logic doesn't make a lot of sense. Can you explain what shared data your mutex protects and what condition the condvar represents? – n. m. could be an AI Feb 08 '23 at 06:20
  • @n.m. i fixed the code sorry. the synchronisation logic here is that the playerthread should random a number but wait until the other threads rnadom and after that all the playerThread until the ManagerGame thread end the current round and start a new round (unlock the mutex and let the players thread to lock and choose number) – Avi C Feb 08 '23 at 10:54
  • One obvious problem is that you are using the same cond var to signal different conditions. There is a separate condition "Player i ready" for all i. Now when yhe manager signals, any player can wake up, and if it's a player whose state is not updated yet, it returns to waiting and this signal is lost forever. – n. m. could be an AI Feb 08 '23 at 12:13
  • But before i do cond_signal from the managerThread i update the state of the players to ready – Avi C Feb 08 '23 at 12:49
  • Sorry misread your code. You are sending *one* signal per round from the manager. This will wake *one* player. You need to wake them all. Perhaps try pthread_cond_broadcast. – n. m. could be an AI Feb 08 '23 at 14:10
  • Ok I’ll try it, but when I after playerThread random number I do signal again, it’s need to signal to another playerThread, isn’t it? Or maybe it signal back to the managerThread and the other thread still waiting? – Avi C Feb 08 '23 at 20:41

2 Answers2

1
void* PlayerFunc(void* playerDetails){
    struct Player player = *((struct Player *)playerDetails);
    ....

PlayerFunc creates a copy of its argument representing that thread/player's state. ManagerGameFunc will never see modifications to that copy, in particular whether it isFinished or not.

pilcrow
  • 56,591
  • 13
  • 94
  • 135
  • So I need in the PlayerFunc to handle the argument like that ‘’’struct Player* player = (struct Player*)playerDetails;’’’? – Avi C Feb 08 '23 at 22:19
  • @AviC, yes, a pointer to the shared/protected state would work and be a natural approach here. – pilcrow Feb 09 '23 at 13:51
0

All your threads follow the same pattern:

    pthread_mutex_lock(&lock);
    while(!predicate(....)){
        pthread_cond_wait(&cond, &lock);
    }
    ....
    pthread_cond_signal(&cond);
    pthread_mutex_unlock(&lock);

First time through the loop all predicates are false. All threads therefore peacefully sleep in pthread_cond_wait. It is never signalled, because neither thread has a chance to progress and signal it. So they wait forever.

user58697
  • 7,808
  • 1
  • 14
  • 28
  • Actually at the start I set to each player of threads player.isFinished to false so the predicate is false and they don’t get in the while block and white – Avi C Feb 08 '23 at 05:50