2

This is my code: Godbolt.

#include <atomic>
#include <iostream>
#include <thread>
#include <vector>

int main(int, char **) {
  volatile bool resource = true;

  std::atomic_bool stop{false};
  std::atomic_int working{0};

  std::thread controller([&]() {
    // Inform all workers resource is going to be destroyed.
    stop.store(true, std::memory_order_relaxed); // #1

    // Wait for all existing workers to finish.
    while (working.load(std::memory_order_relaxed)) { // #2
      std::this_thread::yield();
    }

    // Destroy resource.
    resource = false; // #3
  });

  std::vector<std::thread> workers;
  for (int i = 0; i < 64; ++i) {
    workers.emplace_back([&]() {
      working.fetch_add(1, std::memory_order_relaxed); // #4
      if (stop.load(std::memory_order_relaxed)) { // #5
        working.fetch_sub(1, std::memory_order_relaxed); // #6
        return;
      }
      
      // Access resource
      if (!resource) { // #7
        std::cerr << "Data race detected: resource is not available."
                  << std::endl;
        abort();
      }
      working.fetch_sub(1, std::memory_order_relaxed); // #8
    });
  }

  for (auto &worker : workers) worker.join();
  controller.join();

  std::cout << "no data race detected." << std::endl;
  return 0;
}

The case is like this:

  1. Multiple worker threads accessing a common resource(read-only).
  2. One controller thread will destroy the common resource after informing the workers and wait for the existing workers to finish.

This case describes a common scene: Some workers born at any time, but a controller(resource manager) might intend to destory the resource at any time. To avoid data race, the controller need to wait a bit while for current workers to finish and prevent any new workers.

I used several c++ atomics to get it work. But i have no confidence about the memory ordering although it seems to work well on my x86 server.

  1. In the above code, i just use the relaxed ordering which is apparently not enough, but i don't know which ordering is right here.
  2. By the way, are there any websites or tools to test the memory reordering issues among different platforms?
15
  • 1
    code should be in the question. And often non-working code is not sufficient to explain what the code is supposed to do. You need to explain that too. And apart from the title which is too generic there is no question Commented Jun 29, 2023 at 9:23
  • 3
    Note volatile is NOT related to threadsafety! It is used for memory locations that can be modified by things other then the processor (e.g. interaction with memory mapped hardware) Commented Jun 29, 2023 at 9:23
  • 2
    Welcome to Stack Overflow. Please read the help pages, take the SO tour, and read How to Ask. Then also please read how to write the "perfect" question, especially its checklist. Lastly please try to keep your questions self-contained, with its own minimal reproducible example. Commented Jun 29, 2023 at 9:23
  • 4
    As for your problem, if you have a single common resource then I would recommend you create an object with access functions to the resource. These functions uses a single (shared) mutex to lock the access the resource. Commented Jun 29, 2023 at 9:25
  • 3
    Lockless programming is incredibly difficult to get right. I mean, a condition variable and a mutex would probably even be more performance than your while (working.load(...)) loop Commented Jun 29, 2023 at 9:55

1 Answer 1

1

Briefly: #1, #2, #4, #5 need to be seq_cst. This is the only ordering which prevents StoreLoad reordering (moving a load before a store).

We can see that a data race would potentially occur if #2 were reordered before #1. In that case, it could happen that working.load() returns 0 (all existing workers have finished), but then another worker starts and immediately checks the stop flag (#5), getting the value false. Then it will proceed to access the resource, racing with the controller thread which is now about to destroy it.

Likewise, reordering #4 and #5 also results in a potential data race. Then the worker could see the stop flag as false, but not yet have incremented working to indicate that it has started. If the controller runs at this point, it would proceed to destroy the resource.

Also, #8 needs to be release, since it is essential that it not be reordered before #7. We would similarly argue that #2 needs to be acquire, but in fact it already has to be seq_cst as shown above, which includes all the properties of acquire.

#6 can stay as relaxed. Any thread which reaches #6 is not going to access the resource at all, and so it cannot be involved in a data race.

If I get some time later, I will add a formal proof that these orderings would eliminate the data race.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.