How to safely and properly use threads in C++?

Question

I have a logging system for my application Now this is what i do:

static void Background() {
    while(IsAlive){
         while(!logs.empty()){
             ShowLog(log.front()); 
             log.pop();
         }
         while(logs.empty()){
             Sleep(200);
         }
    }
}

static void Init(){
    // Some Checks
    Logger::backgroundThread = std::thread(Background);
    backgroundThread.detach();
}

static void Log(std::string log){ 
    logs.push(log);
}

static void ShowLog(std::string log){
    // Actual implementation is bit complex but that does not involve threads so i guess it is irrelevant for this question
    std::cout << log << std::endl;
}

Here is a log is a std::queue<std::string>.

Now i am not very sure about whether this is a good approach or not.

Is there any better way to achieve this.

Note i am using C++17

At the very least you need to look into `std::mutex` or a lock-free queue to protect the program from `push`ing and `pop`ping at the same time. — user4581301, May 28 '21 at 05:45
`while(logs.empty()){ Sleep(200); }` - Use a `std::condition_variable` instead of constructs like this. — Ted Lyngmo, May 28 '21 at 05:52
Keep an eye on [this question](https://stackoverflow.com/questions/67733301/consumer-producer-multithreading-freezes) asked a few minutes earlier. It is extremely similar and any answers it gets will be applicable to your case. — user4581301, May 28 '21 at 05:55
Big list of awesome logging libraries for you [here](https://github.com/fffaraz/awesome-cpp#logging) if only for inspiration. — Fantastic Mr Fox, May 28 '21 at 06:01
It isn't about thread safety, but use ```std::move(log)``` in ```void Log``` — Deumaudit, May 28 '21 at 06:04
@FantasticMrFox my logging is mostly external (separate program via network) now i am not much worried about showing the log but i want to make the log queue in my C++ core thread safe for which i guess i need to use std::mutex, std::condition_variable as suggested — Jaysmito Mukherjee, May 28 '21 at 06:04

score 0 · Accepted Answer · answered May 28 '21 at 06:05

0

namespace {  // Anonymous namespace instead of static functions.

std::mutex log_mutex;

void Background() {
    while(IsAlive){
        std::queue<std::string> log_records;
        {
            // Exchange data for minimizing lock time.
            std::unique_lock lock(log_mutex);
            logs.swap(log_records);
        }
        if (log_records.empty()) {
            Sleep(200);
            continue;
        }
        while(!log_records.empty()){
            ShowLog(log_records.front()); 
            log_records.pop();
        }
    }
}

void Log(std::string log){
    std::unique_lock lock(log_mutex);
    logs.push(std::move(log));
}

}

answered May 28 '21 at 06:05

273K

29,503
10
41
64

i just want to say one thing the i placed the sleep there just to reduce CPU usage so if there is a better way you may not use it – Jaysmito Mukherjee May 28 '21 at 06:07
@JaysmitoMukherjee The easy way to replace the sleep is with a [`std::condition_variable`](https://en.cppreference.com/w/cpp/thread/condition_variable). The producer (`Log`) and consumer (`Background`) share a `condition_variable` instance. The consumer waits on the `condition_variable` and after the producer puts an item in the queue, it notifies the consumer with the `condition_variable`. Good example at the bottom of the linked documentation page. – user4581301 May 28 '21 at 15:15
@user4581301 i did change my code to use a condition variable but i dont kown why if i use a condition variable the CPU usage goes upto 10 to 12 percent just for simple logging whereas in this approach it maxes out ad 5 to 6 percent under same conditions – Jaysmito Mukherjee May 29 '21 at 02:43
@JaysmitoMukherjee might be worth asking a new question with your new code. The condition variable approach should result in the logger only waking when it's needed. It's possible you're logging so fast that it's the wrong choice to wake every time, but there's probably a more educational answer worth having in the toolbox. – user4581301 May 29 '21 at 05:23
@user4581301 actually this is not the actual code i have but a highly simplified one in order to focus on what i thought is more important(thread safety). But you are right . This is the logging system of my game engine so logging very fast may be required. And my main priority is to make this program as efficient(fast) as possible so that this does not become a backlog for the engine. and for fast logging i did it . I did the testing with mouse move events(as this should be a lot of logging and the maximum reasonable amount of logging) and with all that mouse logging and sending log over – Jaysmito Mukherjee May 29 '21 at 10:28
@user4581301 the network to my external debugger all that max CPU usage to about 7 -8 percent in all cases. So i guess sleeping in periods then printing a batch that has arrived is better approach rather than walking every time(but i am not very sure and am looking for some better approach). And for a new question i will try to compress the code up so that it can focus on this . – Jaysmito Mukherjee May 29 '21 at 10:31

How to safely and properly use threads in C++?

1 Answers1