CODEWITHSUNDEEP
c++condition-variable
SLI
SLI

Reputation: 753

condition_variable is not working if use it inside structure

condition_variable is not working if use it inside a structure. If I have it as a global variable all works fine. But I need a condition_variable for each packet as I don't know when I will receive an answer and I need to wait for it for each packet. What am I doing wrong?

This is console output:

Wait: 416
StopWait: 423

From it I can see that I receive data and unlock thread after I lock it.

Structures

struct Waiting {
    bool IsWaiting = false;
    mutable std::condition_variable cv;
    mutable std::mutex m;

    clock_t localCLock = 0;

    void Wait() const {
        const double ms = Utils::MillisecondsSpent(localCLock);
        std::cout << "Wait: " << ms << std::endl;

        std::unique_lock<std::mutex> lock(m);
        cv.wait(lock, [this] { return IsWaiting; });
    }

    void StopWait() {
        const double ms = Utils::MillisecondsSpent(localCLock);
        std::cout << "StopWait: " << ms << std::endl;

        std::unique_lock<std::mutex> lock(m);
        IsWaiting = true;
        cv.notify_all();
    }
};

struct Packet
{
    Packet() : id(0), waiting(new Waiting) {}

    int id;
    Waiting* waiting;
};

class Map
{
    static Map* instance;

    Map();
    ~Map();

    Map(const Map&) = delete;

public:
    static Map* Instance() {
        if (!instance) instance = new Map;
        return instance;
    }
    std::map<int, Packet> packets;
};

Threads

//Send Thread - called first
while(true){
    Packet packet;
    packet.id = 1;

    //some send packet logic here
    ...
    ///

    Map::Instance()->packets.insert(std::pair<int, Packet>(p.id, p));
    Map::Instance()->packets[id].waiting->Wait(); // thread now locked and never unlocks

    const Packet received = Map::Instance()->packets[id];
    Map::Instance()->packets.erase(id);
}

//Receive Thread - called second
while(true){
    //some receive packet logic here
    ...
    ///
    const Packet packet = ... // receive a packet data;

    Map::Instance()->packets[packet.id] = packet;
    Map::Instance()->packets[packet.id].answered = true;
    Map::Instance()->packets[packet.id].waiting->StopWait(); // i unlock Send Thread, but it won't work
}

Upvotes: 0

Views: 976

Answers (1)

Rotem
Rotem

Reputation: 21927

Synchronization issues and memory leaks aside, every time you assign a Packet you are copying it by value, and a new Waiting is allocated. There are many different dangling Waiting objects floating around in memory, and there's no reason that calling StopWait on one will trigger the condition_variable on another.

See the code comments I've added.

while(true){
    // *** PACKET A ***
    Packet packet; 
    packet.id = 1;

    //*** PACKET B ***
    Map::Instance()->packets.insert(std::pair<int, Packet>(p.id, p));
    Map::Instance()->packets[id].waiting->Wait();
}

while(true){
    // *** PACKET C ***
    const Packet packet = ... 

    //You are overwriting PACKET B with a copy of PACKET C which is PACKET D.
    //Don't you mean to find a packet which has the same id as the received packet instead of overwriting it?
    Map::Instance()->packets[packet.id] = packet;  
    Map::Instance()->packets[packet.id].answered = true;

    // There's no reason calling StopWait on PACKET D's Waiting object will release PACKET B.
    Map::Instance()->packets[packet.id].waiting->StopWait(); 
}

Upvotes: 1

Related Questions