std::thread communication issue

Question

I am building a multithreaded application, but I've got a problem. In the application I'm using a variable to communicate between the threads but it does not work. So when you type in exit the application does not stop executing.

The code:

//Program that controls an cadrocopter with the use of a
//Raspberry Pi, MPU 6050, an ultrasonic sensor, an HMC5883L and Arduino Leonardo
//to compile use "g++ ./quad.cpp -o ./quad -std=c++0x -pthread"
//
//Copyright Jan Hendrik Farr

#include <iostream>             //used to input data
#include <string>               //used to procces the userdata
#include <stdlib.h>             //used to convert strings into floats
#include "./serial/serial.h"    //used to communicate with the Arduino
#include <thread>               //used to do multithreating

using namespace std;

//userinterface thread
void userInterface(int cmdPos1, float cmdPos[]){
    string cmd = "";

    cout << "************************" << endl;
    cout << "* Quadrocopter control *" << endl;
    cout << "*       software       *" << endl;
    cout << "*                      *" << endl;
    cout << "*     version 0.1      *" << endl;
    cout << "*                      *" << endl;
    cout << "* Copyright J.H. Farr  *" << endl;
    cout << "************************" << endl << endl << endl;

    while(cmdPos1 != 4){
        cin >> cmd;

        if(cmd == "move"){
            cin >> cmdPos[0] >> cmdPos[1] >> cmdPos[2] >> cmdPos[3];
            cmdPos1 = 1;
            cout << endl << endl;
        } else if(cmd == "move+"){
            cin >> cmdPos[0] >> cmdPos[1] >> cmdPos[2] >> cmdPos[3];
            cmdPos1 = 2;
            cout << endl << endl;
        } else if(cmd == "land"){
            cmdPos1 = 3;
            cout << endl << endl;
        } else if(cmd == "exit"){
            cmdPos1 = 4;
            cout << endl;
        } else {
            cout << "invalid argument!!" << endl << endl;
        }
    }
}

//algorithm
void algorithm(float tele[], int cmdPos1, float cmdPos[]){
    while(cmdPos1 != 4){
        switch (cmdPos1){
            case 2:
            cout << "works!!";
            break;

            case 1:

            break;

            case 3:

            break;
        }
    }
}

//gets telemetrydata from mpu
void gettelempu(float tele[], int cmdPos1){
    while(cmdPos1 != 4){

    }
}

//gets height from ultrasonic sensor
void getheight(float tele[], int cmdPos1){
    while(cmdPos1 != 4){

    }
}

//main function
int main(){
    //telemetry data
    float tele[12] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
    //what to do
    int cmdPos1 = 0;
    //where to go
    float cmdPos[4] = {0, 0, 0, 0};

    thread t1(userInterface, cmdPos1, cmdPos);
    thread t2(algorithm, tele, cmdPos1, cmdPos);
    thread t3(gettelempu, tele, cmdPos1);
    thread t4(getheight, tele, cmdPos1);

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

Better not use the `using namespace std`. See [this](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) — Adri C.S., Dec 16 '13 at 14:44
When an integer is given as a function argument by value, a copy if taken. Assigning to "cmdPos1" in one of your threads will only change that local copy, and the other threads cannot see it. You need to use a flag variable which is shared, or even better, used a library with a higher level abstraction than raw threads (which should be rarely needed these days). — , Dec 16 '13 at 14:46
Or you can pass `cmdPos1` by reference to your threads with `std::ref`. — Adri C.S., Dec 16 '13 at 14:47

score 3 · Accepted Answer · answered Dec 16 '13 at 14:49

void gettelempu(float tele[], int cmdPos1)

This function gets its own copy of cmdPos; at the time when it's called, the value passed to it is 0, and the function never changes that value, so the loop never terminates. Same thing in algorithm and getheight.

In order to change the value of this argument in one place and have other functions see that change the functions have to take the value by reference:

void gettelempu(float tele[], int& cmdPos1)

and when the thread is created you have to pass a reference:

thread t3(gettelempu, tele, std::ref(cmdPos1));

But wait, there's more! There's no guarantee that changes to cmdPos1 that are made in one thread will be visible to other threads. To ensure this, make it atomic. In main, change

int cmdPos1 = 0;

to

std::atomic<int> cmdPos1 = 0;

and change the function signatures to take an std::atomic<int> instead of an int:

void gettelempu(float tele[], std::atomic<int>& cmdPos1)

std::thread communication issue

1 Answers1