Mutex as member of class

2

I am trying to make a simple subscriber-producer pattern where multiple subscribers and one producer run in different threads (the reason is to learn more about threading). I am however struggling to make the mutex a member of the producer class. The code is given below:

class Producer {
private:
    vector<Subscriber* > subs;
    thread th;
    int counter;
    mutex mux;

public:
    Producer() : counter(counter), th(&Producer::run, this) {};
    void addSubscriber(Subscriber* s);
    void notify();
    void incrementCounter();
    void run();
    void callJoin(){th.join(); } 
};

void Producer::run() {
    for (int i = 0; i < 10; i++) {
        incrementCounter();
        this_thread::sleep_for(std::chrono::milliseconds(3000));
    }
}

void Producer::addSubscriber(Subscriber* s) {
    lock_guard<mutex> lock(mux);
    subs.push_back(s); 
}

void Producer::notify() {
    lock_guard<mutex> lock(mux);
    for (auto it = subs.begin(); it != subs.end(); ++it) {
        (*it)->setCounterCopy(counter);
    }
}

void Producer::incrementCounter() {
    counter++;
    notify(); 
}

Subscriber class:

class Subscriber {
private:
    string name;
    thread th;
    atomic<int> counterCopy = 0;

public:

    Subscriber(string name) : name(name),  th(&Subscriber::run, this) {};
    void run() {
        while (true) {
            cout << name << ": " << counterCopy << endl; 
            this_thread::sleep_for(std::chrono::milliseconds(1000));            
        }
    }
    void callJoin() { th.join(); }
    void setCounterCopy(int counterCopy) { this->counterCopy = counterCopy; };
};

In main:

int main() {
    Producer p;
    Subscriber s1("Sub1");
    p.addSubscriber(&s1);
    s1.callJoin();
    p.callJoin();
    return 0;
}

The intention of the lock_guard is to prevent the producer from notifying the subscribers in the vector simultaneously as a subscriber is added to the vector. However this exceptation is thrown in the lock_guard of notify Exception thrown at 0x59963734 (msvcp140d.dll) in Project1.exe: 0xC0000005: Access violation reading location 0x00000000. Do anyone know what may be the reason for this exception? If mutex is set as a global parameter this works fine. Feel free to comment other problems of the code also. Threading is all new to me.

c++
multithreading
mutex
asked on Stack Overflow Aug 24, 2018 by (unknown user) • edited Aug 24, 2018 by largest_prime_is_463035818

2 Answers

9

So what's going on here is an initialization order wackiness.

Class members are constructed in the order in which they are declared in the class. In your case, that means first the vector of subscribers, then the thread, then the counter (!), and finally the mutex. The order in which you specify the initializers in the constructor does not matter.

But! Constructing a thread object entails starting the thread, running its function. And that, eventually, results in the mutex being used, potentially before the Producer constructor gets to the point where it actually initializes it. So you end up using a not-yet-constructed mutex, and (not that it's the cause of your issue) also a not-yet-initialized counter.

In general, you should be wary whenever you have a member initializer which mentions this or a different class member, including calling a member function. It sets the scene for accessing an uninitialized object.

In your case, simply moving the mutex and counter members to before the thread member should be sufficient.

answered on Stack Overflow Aug 24, 2018 by Sneftel • edited Sep 23, 2018 by Sneftel
2

I just want to add this to the given answer by @Sneftel for reference:

From CPP standard (N4713), the relevant portion is highlighted:

15.6.2 Initializing bases and members [class.base.init]

...

13 In a non-delegating constructor, initialization proceeds in the following order:
(13.1) — First, and only for the constructor of the most derived class (6.6.2), virtual base classes are initialized in the order they appear on a depth-first left-to-right traversal of the directed acyclic graph of base classes, where “left-to-right” is the order of appearance of the base classes in the derived class base-specifier-list.
(13.2) — Then, direct base classes are initialized in declaration order as they appear in the base-specifier-list (regardless of the order of the mem-initializers).
(13.3) — Then, non-static data members are initialized in the order they were declared in the class definition (again regardless of the order of the mem-initializers).
(13.4) — Finally, the compound-statement of the constructor body is executed.
[ Note: The declaration order is mandated to ensure that base and member subobjects are destroyed in the reverse order of initialization. —end note ]

answered on Stack Overflow Aug 24, 2018 by P.W

User contributions licensed under CC BY-SA 3.0