0

I am learning multithreading and have a simple program below to display some progress. I do not understand why the first code where the function is defined in either the .h file or in the main.cpp file works fine. But if I put the definition of the startLineThread in a new .cpp file like tools.cpp it seems that that second thread never gets a chance to write to the calcProgress variable despite it being and static. the console just loops with zero's and also ignores the calculating bool flag set to false, I can see while debugging after a while the loop in the second loop finishes and prints " done " to the console but the while loop in the main ignores the Tools::calculating == false and keeps looping.

------------------------- tools.h with definition within it ----------------------------
#pragma once
#include <atomic>
#include <iostream>
#include "tools.h"
#include <thread>
#include <chrono>

namespace Tools
{

    static std::atomic<float> clacProgress;
    static volatile bool cFinished;
    static volatile bool calculating;

    void startLineThread()
    {
    std::cout << " second thread active " << std::endl;

    float dataPoints = 1000;
    float dt = 0.5;

    for (float t = 0.0f; t < dataPoints; t++)
    {
        // bs code for making this thread slightly slower maybe
        float no = 87 + 45;
        int slow = 45 * 56 * 23;

        std::this_thread::sleep_for(std::chrono::microseconds(8));

        Tools::clacProgress = t;

    }

    //Tools::cFinished = true;
    Tools::calculating = false;

    std::cout << "done" << std::endl;
}

    
}
 // this way works fine calcProgress is incremented and printed with updated value
// and the while loop in main exits when calculating becomes false;
-------------------------------main.cpp ----------------------------------------
#include <vector>
#include <string>
#include <stdio.h>
#include <math.h>
#include <chrono>
#include <sstream>
#include <thread>
#include "tools.h"
#include <atomic>


int main()
{

    std::cout << " main thread active " << std::endl;

    // initilize all statics before any other threads are active
    //Tools::clacProgress = 0; 
    Tools::calculating = true;
    Tools::cFinished = false;

    std::thread testCalc(Tools::startLineThread);

    while ( Tools::calculating)
    {
        std::cout << Tools::clacProgress << std::endl;
        std::this_thread::sleep_for(std::chrono::microseconds(200));

    }

    std::cin.get();



}

------------------------------------------------------------------------------

Now If I put the definition of startLineThread into a tools.cpp file it breaks everything.

---------------------------- tools.h startLineThread declared but not defined -------
#pragma once
#include <atomic>

namespace Tools
{

    static std::atomic<float> clacProgress;
    static volatile bool cFinished;
    static volatile bool calculating;

    void startLineThread();
    
}

----------------------------- tools.cpp ------------
#include <iostream>
#include "tools.h"
#include <thread>
#include <chrono>

void Tools::startLineThread()
{
    std::cout << " second thread active " << std::endl;

    float dataPoints = 1000;
    float dt = 0.5;

    for (float t = 0.0f; t < dataPoints; t++)
    {
        // bs code for making this thread slightly slower maybe
        float no = 87 + 45;
        int slow = 45 * 56 * 23;

        std::this_thread::sleep_for(std::chrono::microseconds(8));

        Tools::clacProgress = t;

    }

    //Tools::cFinished = true;
    Tools::calculating = false;

    std::cout << "done" << std::endl;
}

 /* this way calcProgress is never incremented in the main thread and prints zero's despite the second thread kicking off and looping and I see " done " printed which indicates it did loop and the while loop in main never exits despite calculating becoming false */

-------------------------------main.cpp ----------------------------------------
#include <vector>
#include <string>
#include <stdio.h>
#include <math.h>
#include <chrono>
#include <sstream>
#include <thread>
#include "tools.h"
#include <atomic>


int main()
{

    std::cout << " main thread active " << std::endl;

    // initilize all statics before any other threads are active
    //Tools::clacProgress = 0; 
    Tools::calculating = true;
    Tools::cFinished = false;

    std::thread testCalc(Tools::startLineThread);

    while ( Tools::calculating)
    {
        std::cout << Tools::clacProgress << std::endl;
        std::this_thread::sleep_for(std::chrono::microseconds(200));

    }

    std::cin.get();



}

------------------------------------------------------------------------------

And IF its a race condition I dont see why I have it in one circumstance and not the other? But I thought the static atomic would prevent the race condition for the threads updating/reporting the progress value

MacLCM
  • 57
  • 9
  • atomic variable just ensure ta what you read and write is not messaed by 2 threads at the same time (consistency). It does not prevent any races in any way. If you need to synchronize thread, you need to do more that just adding atomic stuff. – OznOg Aug 14 '22 at 13:58
  • BTW why do you need volatile variables? – OznOg Aug 14 '22 at 14:01
  • @OznOg I don't need the volatile variable that was a vain attempt to see if it was a work around. originally they were simply static. so about atomic I thought it added first come first serve essentially like if thread1 read , then thread2 writes so that it prevents accessing at the same time. – MacLCM Aug 14 '22 at 14:20

1 Answers1

3

These variables should not be defined in the header file:

static std::atomic<float> clacProgress;
static volatile bool cFinished;
static volatile bool calculating;

By defining them in the header you are making a separate copy of each variable in each .cpp file. Assuming you are using C++17 or newer, change static to inline. If you're using C++14 or older, change static to extern and then define the variables in the .cpp file (so there will be a single copy of each variable, see Declaring variables in header files C++).

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • ooohhh ok but still define them as static in the .cpp file correct? – MacLCM Aug 14 '22 at 14:24
  • I still am getting linking errors now about the externs being unresolved like it can't see the definition in the .cpp file. I simply did static bool Tools::variables; for each of them. – MacLCM Aug 14 '22 at 14:33
  • 1
    No they should not be `static` in the .cpp file. – John Zwinck Aug 14 '22 at 15:39
  • ok that worked thank you very much. so are these variables that are externs like static variables ie only one instance? if so why would we ever use static since as I understand it statics cannot be class members except a static int. this is why I put them in the header in the first place. – MacLCM Aug 14 '22 at 15:58
  • A `static` variable is appropriate in a .cpp file when nothing outside that .cpp file should access it. – John Zwinck Aug 14 '22 at 18:01
  • 1
    I see, so like a private member of a class associated with that .cpp but all instances can access and share the variable. Thank you for taking the time to explain all of this. – MacLCM Aug 16 '22 at 13:14