2

I am having some issue with a process that is being launched by std::async.

class BaseClass {
public:
    BaseClass() {enabledFlag = false;}
    virtual ~BaseClass() {}

protected:
    int process();
    bool enabledFlag;
};


int BaseClass::process() {
    int rc = -1;
    if (enabledFlag == false) {
      std::cout << "Not enabled\n" << std::flush;
      return rc;
    }

    rc = 0;
    while (enabledFlag) {
      // this loop should set rc to be something other than zero if an error is to be signalled
      // otherwise loop here doing stuff until the user sets enabledFlag=false
    }

    return rc;
}

class DerivedClassWithExposedMembersForTesting : public BaseClass {
public:
  using BaseClass::enabledFlag;
  using BaseClass::process;

};

In my Google Test test:

TEST(FixtureName, process_exitsWithRC0_WhenEnabledFlagSetTrueDuringExecution {
    DerivedClassWithExposedMembersForTesting testClass;
    testClass.enabledFlag = true;

    // print status
    std::cout << "Enabled: " <<  testClass.enabledFlag << std::endl << std::flush;

    std::future<int> returnCodeFuture = std::async(std::launch::async, &DerivedClassWithExposedMembersForTesting::process, &testClass);  // starts background execution
    // set flag to false to kill loop
    testClass.enabledFlag = false;

    int rc = returnCodeFuture.get();

    EXPECT_EQ(0, rc);
}

My understanding of std::async is that it should be scheduled to run shortly after the call to async, and the main thread of execution will block at the get() call if the thread hasn't finished. The call to get() will return the return value of process().

process() is set to not run if the testClass is not enabled, hence I am enabling it in the test.

I expect to see:

Enabled: 1
// test passes

What I see is:

Enabled: 1
Not enabled
// test fails
Failure
Value of: rc
  Actual: -1
  Expected: 0

Why is the process triggered by std::async not seeing the value of enabledFlag that is set by the main process prior to the async call being made?

Note: enabledFlag is supposed to be set from an external process, not generally from within the loop, hence this construction

** Update ** As per my comment, I fixed it by adding the following line to the test, just after the call to async():

// Use wait_for() with zero milliseconds to check thread status; delay until it has started
while (returnCodeFuture.wait_for(std::chrono::milliseconds(0)) == std::future_status::deferred) {}
John
  • 10,837
  • 17
  • 78
  • 141
  • 1
    Don't use non-`atomic`s for inter-thread communication. – T.C. Sep 12 '14 at 13:02
  • Sorry, @JoachimPileborg, there was an error in copying the code across. Fixed now. – John Sep 12 '14 at 13:06
  • 1
    Two threads potentially access `testClass.enabledFlag` simultaneously, and one of them modifies it. This is the definition of a *data race*; C++ does not define the behavior of programs with data races. You must synchronize accesses to `testClass.enabledFlag` either with a mutex or by declaring it as an atomic (i.e., `std::atomic`). – Casey Sep 12 '14 at 19:07
  • @Casey, I can see why you need mutex of atomic access when a non-atomic object could be accessed by two threads - one could only partially access the data in the variable whilst the other part is being modified by the other thread. However, how is that an issue for a variable like an integer - surely access to that is inherently atomic, no? How would a thread only partially read an integer? – John Sep 14 '14 at 18:52

1 Answers1

1

The problem is that you don't know when the thread will run. It could be that you simply set the flag to false before the thread actually runs.

One simple way of solving this is to have another state variable, isRunning, that the thread sets inside the loop. Your main thread can check for this to know when the thread is running, and then tell it to end.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Ahhh, of course, thanks. I can see that if I add a delay to the test after the call to async(): for (int i = 0; i < 10000; i++) {} that the code performs as expected. – John Sep 12 '14 at 13:09
  • 2
    Better still than a random delay is to wait only long enough for the thread to start. I added the following line to the test, immediately before setting the enableFlag to false: while (returnCodeFuture.wait_for(std::chrono::milliseconds(0)) == std::future_status::deferred) {} - this is based on a SO answer here: http://stackoverflow.com/questions/9094422/how-to-check-if-a-stdthread-is-still-running – John Sep 12 '14 at 13:23