0

I've got this multithreading project where I'm supposed to create a simulation of a hotel.

I've got this receptionist struct and their job is to constantly find if there's a free room (done in check_free_rooms method, accommodate_guests is his thread method). If he found a number_to_check_in, is_a_room_ready is set to true and one of guest threads waiting for a room is notified.

Every guest has a field which is a reference to the receptionist (there is only one in the hotel) and the guests are waiting on the receptionist's condition variable receptionist.cv to be notified with the condition of receptionist.is_a_room_ready becoming true. Then, if I understand correctly, one random guest should get a room and the other ones should patiently wait for another notification from the receptionist.

#include <iostream>
#include <vector>
#include <thread>
#include <mutex>
#include <chrono>
#include <algorithm>
#include <experimental/random>
#include <atomic>
#include <ncurses.h>
#include <condition_variable>
#include <memory>

std::mutex mx_writing;

struct Guest;

struct Room
{
    Room() {}
    int id;
    int guest_id;
    std::atomic<bool> is_ready_for_guest{true};

    void guest_arrives(int guest_id)
    {
        this->guest_id = guest_id;
        this->is_ready_for_guest = false;
    }
    void guest_leaves(int guest_id)
    {
        this->guest_id = -1;
    }
};

struct Receptionist
{
    Receptionist(std::vector<Room> &rooms) : rooms(rooms) {}
    std::vector<Room> &rooms;

    std::mutex mx;
    std::condition_variable cv;
    std::atomic<bool> is_a_room_ready{false};
    int number_to_check_in = 0;

    void check_free_rooms()
    {
        std::unique_lock<std::mutex> lock_receptionist(mx);
        do
        {
            this->number_to_check_in = std::experimental::randint(0, (int)rooms.size() - 1); //find an empty room
        } while (!rooms[this->number_to_check_in].is_ready_for_guest);
        is_a_room_ready = true;
        cv.notify_one();
    }

    void accommodate_guests()
    {
        while (true)
        {
            check_free_rooms();
            std::this_thread::sleep_for(std::chrono::milliseconds(100));
        }
    }
};

struct Guest
{
    Guest(int id, Receptionist &receptionist, Coffee_machine &coffee_machine,
          Swimming_pool &swimming_pool) : id(id), receptionist(receptionist),
                                          coffee_machine(coffee_machine), swimming_pool(swimming_pool) {}
    int id;
    int room_id;

    Receptionist &receptionist;
    Coffee_machine &coffee_machine;
    Swimming_pool &swimming_pool;

    void check_in()
    {
        std::unique_lock<std::mutex> lock_receptionist(receptionist.mx);
        while (!receptionist.is_a_room_ready)
        {
            receptionist.cv.wait(lock_receptionist);
        }
        receptionist.is_a_room_ready = false;
        this->room_id = receptionist.number_to_check_in;           //assign room to guest
        receptionist.rooms[this->room_id].guest_arrives(this->id); //assign guest to room && room becomes occupied
        {
            std::lock_guard<std::mutex> writing_lock(mx_writing);
            std::cout << "Guest " << this->id << " accomodated in room " << this->room_id << std::endl;
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(20));
    }

    void have_holiday()
    {
        check_in();
        std::this_thread::sleep_for(std::chrono::milliseconds(std::experimental::randint(500, 700)));
    }
};

int main()
{
    std::vector<Room> rooms(10);
    for (int i = 0; i < 10; i++)
    {
        rooms[i].id = i;
    }

    Receptionist receptionist(rooms);

    std::vector<Guest> guests;
    for (int i = 0; i < 15; i++)
    {
        guests.emplace_back(Guest(i, receptionist, coffee_machine, swimming_pool));
    }

    std::vector<std::thread> threadList;

    threadList.emplace_back(std::thread(&Receptionist::accommodate_guests, std::ref(receptionist)));
    for (Guest guest : guests)
    {
        threadList.emplace_back(std::thread(&Guest::have_holiday, std::ref(guest)));
    }

    for (std::thread &t : threadList)
    {
        t.join();
    }

    return 0;
}

However, the lines that appear in the terminal after a guest gets their room are not quite what I imagined. Guest ids range from 0 to 14 and room ids range from 0 to 9 but only rooms seem to be more or less ok in the outputs. I've got no idea why guest ids are a random large int and not the ones I assigned while object construction.

I'm not that very experienced with multithreading and condition variables and have virtually no clue how to solve this problem as I gave it a lot of thinking and came up with nothing. I'd very much appreciate any help.

Some example outputs:

Guest 5 accommodated in room 32657
Guest -624431120 accommodated in room 7
Guest -624431120 accommodated in room 9
Guest -624431120 accommodated in room 8
Guest -624431120 accommodated in room 5
Guest -624431120 accommodated in room 4
Guest -624431120 accommodated in room 3
Guest -624431120 accommodated in room 2
Guest -624431120 accommodated in room 0
Guest -624431120 accommodated in room 6
^C

or

Guest 4 accommodated in room 32539
Guest -497561616 accommodated in room 1
Guest -497561616 accommodated in room 9
Guest -497561616 accommodated in room 5
Guest -497561616 accommodated in room 8
Guest -497561616 accommodated in room 6
Guest -497561616 accommodated in room 7
Guest -497561616 accommodated in room 3
Guest -497561616 accommodated in room 4
Guest -497561616 accommodated in room 0
^C

or

Guest 4 accommodated in room 32746
Guest -1510756368 accommodated in room 8
Guest -1510756368 accommodated in room 1
Guest -1510756368 accommodated in room 7
Guest -1510756368 accommodated in room 4
Guest -1510756368 accommodated in room 2
Guest -1510756368 accommodated in room 5
Guest -1510756368 accommodated in room 9
Guest -1510756368 accommodated in room 6
Guest -1510756368 accommodated in room 3
^C
8
  • Where is mx_writing defined? Commented May 23, 2020 at 18:07
  • for (Guest guest : guests) you are making a copy of guest here, and taking a reference to it? Commented May 23, 2020 at 18:08
  • @KorelK Globally, I edited the question Commented May 23, 2020 at 18:10
  • 1
    Yes, try for (Guest &guest : guests) and see what happens. Commented May 23, 2020 at 18:13
  • 1
    The & means guest is a reference to every element in guests. This is usually what you want, so prefer this syntax. In fact, do for (Guest const &guest : guests) when you don't want to change the elements at all. Commented May 23, 2020 at 18:22

1 Answer 1

2

Replace:

for (Guest guest : guests) {
    threadList.emplace_back(std::thread(&Guest::have_holiday, std::ref(guest)));
}

With:

for (Guest &guest : guests) {
    threadList.emplace_back(std::thread(&Guest::have_holiday, std::ref(guest)));
}

In your code, you are creating a copy of guest in every iteration instead of using the existing one.
Pay attention to Guest guest [copy] vs Guest &guest [reference].
Read about references in C++.

Another Fix:

for (int i = 0; i < 15; i++) {
    guests.emplace_back(Guest(i, receptionist, coffee_machine, swimming_pool));
}

In this section, you are creating the guest object twice in each iteration. In emplace_back you can just pass the parameters for the constructor without creating a copy:

for (int i = 0; i < 15; i++) {
    guests.emplace_back(i, receptionist, coffee_machine, swimming_pool);
}
Sign up to request clarification or add additional context in comments.

4 Comments

Could you explain why the second snippet is not making a copy?
Much better, thanks. Note that another fix is not really a "fix", but perhaps an "improvement".
Thanks a lot for the explanation (also provided by @cigien) and for another improvement suggestion.
@Theta no problem. About the improvement suggestion, pay attention that it holds also for threadList.emplace_back(&Receptionist::accommodate_guests, std::ref(receptionist)); and for threadList.emplace_back(&Guest::have_holiday, std::ref(guest));.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.