Willingly introducing a race condition on a 64-bit pointer value

Viewed 164

I have an application object which can receive messages from multiple services running in multiple threads. The message gets dispatched internally by an instance of a dispatcher object in the threads of the services. The application can at any time change the current dispatcher. Dispatchers never get destroyed. The services never outlive the application.

Here's an example code

#include <iostream>
#include <thread>
#include <atomic>
#include <cstdlib>
#include <functional>

using namespace std;

using Msg = int;

struct Dispatcher
{
    virtual ~Dispatcher() = default;
    virtual void dispatchMessage(Msg msg) = 0;
};

struct DispatcherA : Dispatcher
{
    void dispatchMessage(Msg msg)
    {
        cout << "Thread-safe dispatch of " << msg << " by A" << endl;
    }
};

struct DispatcherB : Dispatcher
{
    void dispatchMessage(Msg msg)
    {
        cout << "Thread-safe dispatch of " << msg << " by B" << endl;
    }
};

struct Application
{
    Application() : curDispatcher(&a) {}
    void sendMessage(Msg msg)
    {
        // race here as this is called (and dereferenced) from many threads
        // and can be changed by the main thread
        curDispatcher->dispatchMessage(msg);
    }

    void changeDispatcher()
    {
        // race her as this is changed but can be dereferenced by many threads
        if (rand() % 2) curDispatcher = &a;
        else curDispatcher = &b;
    }

    atomic_bool running = true;

    Dispatcher* curDispatcher; // race on this
    DispatcherA a;
    DispatcherB b;
};

void service(Application& app, int i) {
    while (app.running) app.sendMessage(i++);
}

int main()
{
    Application app;
    std::thread t1(std::bind(service, std::ref(app), 1));
    std::thread t2(std::bind(service, std::ref(app), 20));

    for (int i = 0; i < 10000; ++i) 
    {
        app.changeDispatcher();
    }
    app.running = false;

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

I am aware that there is a race condition here. The curDispatcher pointer gets accessed by many threads and it can be changed at the same time by the main thread. It can be fixed by making the pointer atomic and explicitly loading it on every sendMessage call.

I don't want to pay the price of the atomic loads.

Can something bad happen of this?

Here's what I can think of:

  • The value of curDispatcher can get cached by a service and it can always call the same one, even if the app has changed the value. I'm ok with that. If I stop being ok with that, I can make it volatile. Newly created services should be ok, anyway.
  • If this ever runs on a 32-bit CPU which emulates 64-bit, the writes and reads of the pointer will not be instruction-level atomic and it might lead to invalid pointer values and crashes: I am making sure that this only runs on 64-bit CPUs.
  • Destroying dispatchers isn't safe. As I said: I'm never destroying dispatchers.
  • ???
0 Answers
Related