C++ values not updating from thread

Viewed 231

I have made the following struct:

struct CLICKING {
    std::chrono::system_clock::time_point lastClickTime;

    void clickMouse() {
        /* click mouse stuff here */
        lastClickTime = std::chrono::system_clock::now();
        std::cout << "Setting last click time to " << std::chrono::system_clock::to_time_t(lastClickTime) << std::endl;
    }
};

In my main class, I create the struct:

std::unique_ptr<CLICKING> clickingPtr = std::make_unique<CLICKING>();
clickingPtr->lastClickTime = std::chrono::system_clock::now();

And then in a Windows thread (using CreateThread), I have something like this (in an infinite loop):

std::cout << "Last click time in thread: " << std::chrono::system_clock::to_time_t(inst->getClickDetails().lastClickTime) << std::endl;
auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - inst->getClickDetails().lastClickTime).count();

....

// At some point, if factors are correct, click the mouse:
inst->getClickDetails().clickMouse();

inst refers to this, just passed into the thread, and getClickDetails() grabs the unique_ptr. However, after all of this, I get the same thing printed out: Last click time in thread: 1592999354

but every time I do call for clickMouse() I can see: Setting last click time to 1592999377.

It seems that lastClickTime is only being set the first time (when creating the unique_ptr) but on subsequent calls to set it it updates within the struct (as seen from the struct printout) but asking for its value in the thread it does not update...

Any ideas?

1 Answers

Long story short - you are in realms of undefined behaviour. Why? Because you are causing data race by reading and writing to unsynchronized memory via multiple threads. What it means? It means your program may behave in completely unpredictable ways which happened as you noticed posting it here. How to fix it? One possibility is using std::mutex like that:

struct CLICKING {
    std::chrono::system_clock::time_point lastClickTime;
    std::mutex mtx;

    void clickMouse() {
        /* click mouse stuff here */
        std::lock_guard<std::mutex> l{mtx};
        lastClickTime = std::chrono::system_clock::now();
        std::cout << "Setting last click time to " << std::chrono::system_clock::to_time_t(lastClickTime) << std::endl;
    }
};

std::cout << "Last click time in thread: " << 
std::lock_guard<std::mutex> l{inst->getClickDetails().mtx};
std::chrono::system_clock::to_time_t(inst->getClickDetails().lastClickTime) << std::endl;
auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - inst->getClickDetails().lastClickTime).count();
Related