Thread safe std::cout

Question

The following program still interleaves the output to std::cout. I tried to add a std::mutex to control access to std::cout via std::lock_guard, but it still interleaves.

#include <iostream>
#include <chrono>
#include <thread>
#include <functional>
#include <mutex>
#include <condition_variable>

std::mutex global_mtx{};

class Timer {
public:
    Timer(size_t time, const std::function<void(void)>& f) : time{std::chrono::milliseconds{time}}, f{f} {}
    ~Timer() { wait_thread.join(); }

private:
    void wait_then_call()
    {
        std::unique_lock<std::mutex> lck{mtx};
        for(int i{10}; i > 0; --i) {
            {
                std::lock_guard<std::mutex>{global_mtx};
                std::cout << "Thread " << wait_thread.get_id() << " countdown at: " << '\t' << i << std::endl;

            }
            cv.wait_for(lck, time / 10);
        }
        f();
    }
    std::mutex mtx;
    std::condition_variable cv{};
    std::chrono::milliseconds time;
    std::function <void(void)> f;
    std::thread wait_thread{[this]() {wait_then_call(); }};
};

int main()
{
    auto f = []() {std::lock_guard<std::mutex>{global_mtx}; std::cout << "---------------- I waited to print! ----------------" << std::endl; };
    Timer t1{3'000,f};
    Timer t2{6'000,f};
    Timer t3{2'000,f};
    Timer t4{1'000,f};
}

Do I need to control access through a separate class or dedicated thread?

Answer 1

One way to prevent forgetting to name the lock is to make a lock object that you can use as an io manipulator:

#include <iostream>
#include <chrono>
#include <thread>
#include <functional>
#include <mutex>
#include <condition_variable>

std::mutex global_mtx{};

struct lockio
{
    lockio(std::mutex& m) : lock_(m) {}

    std::unique_lock<std::mutex> lock_;
};
std::ostream& operator<<(std::ostream& os, const lockio&) {
    return os;
}

class Timer {
public:
    Timer(size_t time, const std::function<void(void)>& f) : time{std::chrono::milliseconds{time}}, f{f} {}
    ~Timer() { wait_thread.join(); }

private:
    void wait_then_call()
    {
        std::unique_lock<std::mutex> lck{mtx};
        for(int i{10}; i > 0; --i) {
            {
                std::cout << lockio(global_mtx) << "Thread " << wait_thread.get_id() << " countdown at: " << '\t' << i << std::endl;

            }
            cv.wait_for(lck, time / 10);
        }
        f();
    }
    std::mutex mtx;
    std::condition_variable cv{};
    std::chrono::milliseconds time;
    std::function <void(void)> f;
    std::thread wait_thread{[this]() {wait_then_call(); }};
};

int main()
{
    auto f = []() { std::cout << lockio(global_mtx) << "---------------- I waited to print! ----------------" << std::endl; };
    Timer t1{3'000,f};
    Timer t2{6'000,f};
    Timer t3{2'000,f};
    Timer t4{1'000,f};
}

Another (probably better) way is to create a little helper template function to wrap the protected operations:

#include <iostream>
#include <thread>
#include <condition_variable>

std::mutex global_mtx{};

template<class Mutex, class F>
decltype(auto) with_lock(Mutex &m, F &&f) {
    std::lock_guard<Mutex> lock(m);
    return f();
};

class Timer {
public:
    Timer(size_t time, const std::function<void(void)> &f) : time{std::chrono::milliseconds{time}}, f{f} {}

    ~Timer() { wait_thread.join(); }

private:
    void wait_then_call() {
        std::unique_lock<std::mutex> lck{mtx};
        for (int i{10}; i > 0; --i) {
            with_lock(global_mtx, [&] {
                std::cout << "Thread " << wait_thread.get_id() << " countdown at: " << '\t' << i << std::endl;
            });
            cv.wait_for(lck, time / 10);
        }
        f();
    }

    std::mutex mtx;
    std::condition_variable cv{};
    std::chrono::milliseconds time;
    std::function<void(void)> f;
    std::thread wait_thread{[this]() { wait_then_call(); }};
};

int main() {
    auto f = []() {
        with_lock(global_mtx, []
        {
            std::cout << "---------------- I waited to print! ----------------" << std::endl;
        });
    };
    Timer t1{3'000, f};
    Timer t2{6'000, f};
    Timer t3{2'000, f};
    Timer t4{1'000, f};
}

one more way:

#include <iostream>
#include <thread>
#include <condition_variable>


struct locked {

    std::ostream& cout() const { return std::cout; }
    std::ostream& cerr() const { return std::cerr; }

private:
    static std::mutex& mutex() {
        static std::mutex stdio_mutex;
        return stdio_mutex;
    }
    std::unique_lock<std::mutex> lock_{mutex()};
};

class Timer {
public:
    Timer(size_t time, const std::function<void(void)> &f) : time{std::chrono::milliseconds{time}}, f{f} {}

    ~Timer() { wait_thread.join(); }

private:
    void wait_then_call() {
        std::unique_lock<std::mutex> lck{mtx};
        for (int i{10}; i > 0; --i) {
            locked().cout() << "Thread " << wait_thread.get_id() << " countdown at: " << '\t' << i << std::endl;
            cv.wait_for(lck, time / 10);
        }
        f();
    }

    std::mutex mtx;
    std::condition_variable cv{};
    std::chrono::milliseconds time;
    std::function<void(void)> f;
    std::thread wait_thread{[this]() { wait_then_call(); }};
};

int main() {
    auto f = []() {
        locked().cout() << "---------------- I waited to print! ----------------" << std::endl;
    };
    Timer t1{3'000, f};
    Timer t2{6'000, f};
    Timer t3{2'000, f};
    Timer t4{1'000, f};
}

Answer 2

You create four Timer objects, each one having its own unique mutex object. So when t2 runs its thread, it locks its own mutex and it can because t1 locked a different mutex before beginning its loop.

Answer 3

Your problem is here: std::lock_guard<std::mutex>{global_mtx}; creates a lock guard and immediately releases it. You need to create a variable to hold the lock, like std::lock_guard<std::mutex> lock{global_mtx};.