Shared variables in two threads-CodePudding

I'm writing a ros node with two threads to support client dynamic reconfiguration in C . One thread is responsible for subscribing to a topic /error which is float, taking the sum of value sum_of_error, and incrementing batch_size. Another thread is responsible for taking the average of the value if batch_size reaches BATCH and then resetting sum_of_error and batch_size to 0. Both batch_size and sum_of_error are shared variables between two threads so I use mutex to prevent data race. The thread for subscription of a topic works well but the thread for taking the average and resetting batch_size and sum_of_error is stuck. I notice that the process gets stuck at while loop in the function get_value(). I think it has something to do with race conditions. Any idea? How can I change my code to solve this problem.

#include <ros/ros.h>
#include <string>
#include <vector>
#include <std_msgs/Float32.h>
#include <dynamic_reconfigure/client.h>
#include <calibration/HcNodeConfig.h>
#include <mutex>
#include <thread>

using namespace std;


mutex m;
int batch_size = 0;
float sum_of_error = 0;

int BATCH = 10;


float get_value() {
    while (batch_size < BATCH) {
    }
    float res = sum_of_error / batch_size;
    m.lock();
    sum_of_error = 0.0;
    batch_size = 0;
    m.unlock();
    return res;
}


void adjuster_callback(const hc::HcNodeConfig& data) {
    
}


void listener_callback(const std_msgs::Float32ConstPtr& msg) {
    float current_error = msg->data;
    m.lock();
    batch_size  ;
    sum_of_error  = current_error;
    m.unlock();
}


void start_listener(ros::NodeHandle n) {
    ros::Subscriber listener;
    ros::Rate loop_rate(BATCH);
    listener = n.subscribe("/error", BATCH, listener_callback);
    while (ros::ok()) {
        ros::spinOnce();
        loop_rate.sleep();
    }
}


void start_adjuster(ros::NodeHandle n) {
    ros::Rate loop_rate(BATCH);
    hc::HcNodeConfig config;
    dynamic_reconfigure::Client<hc::HcNodeConfig> client("/lidar_front_left_hc", adjuster_callback);
    ros::Duration d(2);
    client.getDefaultConfiguration(config, d);
    while (ros::ok()) {
        float current_error = get_value();
        if (current_error > 0.0) {
             config.angle = 0.0;
             client.setConfiguration(config);
        }
        ros::spinOnce();
        loop_rate.sleep();
    }
}


int main(int argc, char ** argv) {
    ros::init(argc, argv, "adjuster");
    ros::NodeHandle n;

    thread t1(start_listener, n);
    thread t2(start_adjuster, n);

    t1.join();
    t2.join();
    return 0;
}

CodePudding user response：

    while (batch_size < BATCH) {
    }

The apparent intent of this is to wait until another execution thread increments batch_size until it reaches BATCH.

However because this access to batch_size is not synchronized, at all, this results in undefined behavior.

Other execution threads synchronize modifications to batch_size using the mutex. This execution thread must do the same. Something like this:

int get_batch_size()
{
    m.lock();

    int s=batch_size;

    m.unlock();

    return s;
}

// ...

while (get_batch_size() < BATCH)
   ;

This is, of course, busy-polling and would be horribly inefficient. A condition variable should be employed, together with this mutex, for optimal performance and results.