How bad it is to lock a mutex in an infinite loop or an update function-CodePudding

std::queue<double> some_q;
std::mutex mu_q;

/* an update function may be an event observer */
void UpdateFunc()
{
    /* some other processing */
    std::lock_guard lock{ mu_q };
    while (!some_q.empty())
    {
        const auto& val = some_q.front();
        /* update different states according to val */
        some_q.pop();
    }
    /* some other processing */
}

/* some other thread might add some values after processing some other inputs */
void AddVal(...)
{
    std::lock_guard lock{ mu_q };
    some_q.push(...);
}

For this case is it okay to handle the queue this way? Or would it be better if I try to use a lock-free queue like the boost one?

CodePudding user response：

How bad it is to lock a mutex in an infinite loop or an update function

It's pretty bad. Infinite loops actually make your program have undefined behavior. There must be an exit. Also, when holding the mutex, noone can add information to the queue, so while processing the information you extract, all threads wanting to add to the queue will have to wait - and no other worker threads wanting to share the load can extract from the queue either. It's usually better to extract one task from the queue, release the lock and then work with what you got.

The common way is to use a condition_variable that lets other threads acquire the lock and then notify other threads waiting with the same condition_variable. The CPU will be pretty close to idle while waiting and wake up to do the work when needed.

Using your program as a base, it could look like this:

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

std::queue<double> some_q;
std::mutex mu_q;
std::condition_variable cv_q; // the condition variable
bool stop_q = false;          // something to signal the worker thread to quit

/* an update function may be an event observer */
void UpdateFunc() {
    while(true) {
        double val;
        {
            std::unique_lock lock{mu_q};

            // cv_q.wait lets others acquire the lock to work with the queue
            // while it waits to be notified.
            while (not stop_q && some_q.empty()) cv_q.wait(lock);

            if(stop_q) break; // time to quit

            val = std::move(some_q.front());
            some_q.pop();
        } // lock released so others can use the queue

        // do time consuming work with "val" here
        std::cout << "got " << val << '\n';
    }
}

/* some other thread might add some values after processing some other inputs */
void AddVal(double val) {
    std::lock_guard lock{mu_q};
    some_q.push(val);
    cv_q.notify_one(); // notify someone that there's a new value to work with
}

void StopQ() { // a function to set the queue in shutdown mode
    std::lock_guard lock{mu_q};
    stop_q = true;
    cv_q.notify_all(); // notify all that it's time to stop
}

int main() {
    auto th = std::thread(UpdateFunc);
    
    // simulate some events coming with some time apart
    std::this_thread::sleep_for(std::chrono::seconds(1));
    AddVal(1.2);
    std::this_thread::sleep_for(std::chrono::seconds(1));
    AddVal(3.4);
    std::this_thread::sleep_for(std::chrono::seconds(1));
    AddVal(5.6);
    std::this_thread::sleep_for(std::chrono::seconds(1));

    StopQ();    
    th.join();
}

CodePudding user response：

You can use it like you currently are assuming proper use of the lock across all threads. However, you may run into some frustrations about how you want to call updateFunc().

Are you going to be using a callback?
Are you going to be using an ISR?
Are you going to be polling?

If you use a 3rd party lib it often trivializes thread synchronization and queues

For example, if you are using a CMSIS RTOS(v2). It is a fairly straight forward process to get multiple threads to pass information between each other. You could have multiple producers, and a single consumer.

The single consumer can wait in a forever loop where it waits to receive a message before performing its work

when timeout is set to osWaitForever the function will wait for an infinite time until the message is retrieved (i.e. wait semantics).

// Two producers
osMessageQueuePut(X,Y,Z,timeout=0)
osMessageQueuePut(X,Y,Z,timeout=0)
    
// One consumer which will run only once something enters the queue
osMessageQueueGet(X,Y,Z,osWaitForever)

tldr; You are safe to proceed, but using a library will likely make your synchronization problems easier.