CODEWITHSUNDEEP
c++multithreading
Jarek
Jarek

Reputation: 235

Misuse of conditional variable

Could you please review and suggest what is wrong with this code? It either crashes on line 21 (cond_var_.wait(lock); in the gc_thread_proc()) or locks on line 56 (lock.lock(); in release()).

#include <condition_variable>
#include <deque>
#include <functional>
#include <mutex>
#include <thread>
#include <vector>
#include <iostream>

class stream {
    std::deque<int> pending_cleanups_;
    std::mutex mut_{};
    bool continue_{true};
    std::thread gc_worker_;
    std::condition_variable cond_var_;

    void gc_thread_proc() {
        while (true) {
            std::vector<int> events_to_clean;
            std::unique_lock<std::mutex> lock(mut_);
            while (pending_cleanups_.empty() && continue_) {
                cond_var_.wait(lock);
            }
            if (!continue_) {
                break;
            }
            std::move(std::begin(pending_cleanups_), std::end(pending_cleanups_), std::back_inserter(events_to_clean));
            pending_cleanups_.clear();
        }
    }


public:
    explicit stream() : gc_worker_(&stream::gc_thread_proc, this) {}

    void register_pending_event(int val) {
        {
            std::lock_guard<std::mutex> lock_guard(mut_);
            pending_cleanups_.push_back(val);
        }
        cond_var_.notify_one();
    }

    void release() {
        std::unique_lock<std::mutex> lock(mut_);
        if (!continue_) {
            return;
        }
        continue_ = false;
        lock.unlock();
        cond_var_.notify_one();
        gc_worker_.join();
        lock.lock();
        pending_cleanups_.clear();
    }

    ~stream() { release(); }
};


int main() {
    int N=100000;
    while(N--) {
        std::cout << ".";
        stream s;
    }
    std::cout << "ok";

    return 0;
}

Changing order of members makes this problem go away - when cond_var_ is put before the gc_worker_ problem doesn't reproduce. But I guess it doesn't fix it just hides it somehow...

Upvotes: 1

Views: 84

Answers (1)

SPD
SPD

Reputation: 388

non-static data members are initialized in order of declaration in the class definition: https://en.cppreference.com/w/cpp/language/initializer_list

3) Then, non-static data members are initialized in order of declaration in the class definition.

In your case, since your std::thread member is initialized to start executing in its constructor, cv may not be initialized when it's used in gc_thread_proc. A command way to have a std::thread member is to move assign it in the class contructor, i.e.

class stream {
    std::thread gc_worker_;
    std::condition_variable cond_var_;
public:
  stream(): {
     gc_work = std::move(std::thread(&stream::gc_thread_proc, this));
  }
};

Upvotes: 3

Related Questions