2

tl;dr:

 class Controller
 {
 public:
    volatile Netconsole* nc;
    void init();        //initialize the threads
    void calculate();   // handler for the "mothership app"
    void senderThreadLoop();  //also calls reinitNet() if connection is broken.
    void listenerThreadLoop();
    inline void reinitNet(){ delete nc; nc = new Netconsole(); }
 }

// inside Json::Value header = nc->Recv();

error: passing 'volatile Netconsole' as 'this' argument discards qualifiers [-fpermissive]

Pointer to an instance of a utility class (Netconsole) shared between two threads must be updated inside both threads if the utility class is re-instantiated, but declaring it as volatile generates the above error. If it's updated just inside one thread, the other thread may still use old, invalid pointer. How to assure it's updated in both but using methods through the pointer doesn't trigger the above error?

Extended info:

The "smart glue logic" library I'm writing is used to pass and convert messages between a 3rd party software and a custom device. It consists of three essential threads:

  • a handler: the main thread of the 3rd party app periodically calls a "calculate" function in my library to handle new updates - data to send, data received
  • a sender thread that converts and sends whatever the handler pushed into the send buffer
  • a listener thread that converts and pushes any data received from the device into receive buffer.

Both the sender and the listener threads use the same utility class that handles network communication with the device; upon initialization the class creates a connection to the device, and the two threads perform blocking reads or await for new data to send respectively. In case of any problems, the sender thread performs all "maintenance" work, while the listener thread enters a safe state awaiting return of connectivity.

Now, since the two threads share one connection to the device, they both share the same instance of the communication class, as a pointer to that class.

The problem is in the procedure of reconnect - it involves destroying and creating the helper class instance exploiting safe shutdown and initialization already present in the destructor and constructor. As result the pointer changes. Without volatile it's quite likely the listener won't receive the updated pointer. With volatile, it protests - needlessly, because nc (the pointer) won't change at a random moment - first the listener is notified of a problem, then it enters a safe state where it doesn't perform any operations on 'nc' and notifies the sender it's ready. Only then the sender performs the repair and notifies the listener to resume normal operation.

So what's the right solution in this situation?

SF.
  • 13,549
  • 14
  • 71
  • 107
  • 3
    You cold use a [`std::atomic`](http://en.cppreference.com/w/cpp/atomic/atomic) – NathanOliver Sep 14 '15 at 14:44
  • 4
    `volatile` isn't made for thread safety (and won't accomplish it). Use a `std::mutex` to protect it from race conditions. – πάντα ῥεῖ Sep 14 '15 at 14:45
  • Besides, it would have to be `Netconsole* volatile nc;` to accomplish the stated goal even if `volatile` would have meant thread-safe. – MSalters Sep 14 '15 at 14:49
  • @MSalters: Mutex can be used to stop and start threads, not to pass data from a thread to another. It's not atomicity/race condition problem here but problem of caching/optimizing variables. – SF. Sep 14 '15 at 14:53
  • (take this example: `int x=0; thread1(){ myMutex.lock(); x = 1 ; myMutex.unlock(); } thread2(){ int y=0; myMutex.lock(); y = x ; myMutex.unlock(); cout << y < – SF. Sep 14 '15 at 14:55
  • @SF. You are incorrect, See: http://stackoverflow.com/questions/3208060/does-guarding-a-variable-with-a-pthread-mutex-guarantee-its-also-not-cached and https://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/ – NathanOliver Sep 14 '15 at 15:03
  • @SF: You misdirected the comment, but πάντα ῥεῖ is right. A mutex doesn't _share_ data, but the associated memory barrier ensures that shared data is _visible_ to the other thread. The `std::atomic` class is useful because its `.load` method allows you to specify a memory barrier independent from a mutex. – MSalters Sep 14 '15 at 15:06
  • SF, there is a simple rule of thumb. If you are using volatile because you need it for thread communication, you are wrong (unless you are on Intel and know what you are doing). – SergeyA Sep 14 '15 at 15:06
  • @SergeyA: Unless you're on _Visual Studio_ and you know what you're doing. It's a Microsoft extension. I don't think ICC makes the same guarantee. – MSalters Sep 14 '15 at 15:09
  • @MSalters, it is not VisualStudio which does this gurantee, it is Intel. Since compiler guarantees that all reads from volatile are, in fact, reads (not cached) and intel gurantees cache corehence and read/write atomicity for aligned data, on Intel voliatile will act as atomic regardless of compiler. – SergeyA Sep 14 '15 at 15:11
  • @SergeyA: Well, Microsoft guarantees it on ARM, and Intel doesn't guarantee it on SSE streaming load/stores (which happen to be used in some `memcpy` implementations). Furthermore, Intel only guarantees it up to 8 bytes IIRC. – MSalters Sep 14 '15 at 15:20

1 Answers1

1

What you need is a sequence of operations. The producing thread has 2 relevant operations : "initialize new Netconsole" and "write pointer". The consuming thread also has two operations: "read pointer" and "use new Netconsole object". Those 4 operations must be sequenced in exactly that order for the update to be visible.

By far the simplest way to achieve this are two memory barriers. A write barrier (std::memory_order_release on the pointer write) prevents the first two operations from being reordered, and the read barrier (std::memory_order_acquire on the pointer load) prevents the last two operations from being reordered.

As the two threads run independently, your program correctness shouldn't depend on whether a particular object update happened before a particular object use. The updating thread might just have been a bit slow, and that should not break your program. So the third ordering between write and read isn't really relevant and you shouldn't try to "fix" it.

To summarize: Yes, the 4 operations have to happen in exactly the right order for the result to be visible, but if the second and third operation are reordered then the update is perfectly invisible to the consuming thread. It's an atomic update, all or nothing.

There's still a matter of cleaning up the old object. The producing thread cannot just assume that the consuming thread has already seen the pointer update. There must be synchronization to ensure both threads agree that the old object is unused. The easiest is if the producing thread strictly does not use the old object after the new object has been created (the memory barrier helps here), and the consuming thread cleans up the old object as soon as it knows there's a new object (because that happens strictly after the read barrier, thus after the write barrier and in turn after the last use by the producing thread)

MSalters
  • 173,980
  • 10
  • 155
  • 350