Is volatile variables safe in one customer thread and one producer thread?

Question

is this code correct? I see this code in someone's blog, it said that volatile is safe in environment of only one customer and only one producer. I don't know if it's really thread-safe.

the code is following:

#include <iostream>
#include <pthread.h>


template<class QElmType>
struct qnode
{
    struct qnode *next;
    QElmType data;
};
template<class QElmType>
class queue
{
public:
        queue() {init();}
        ~queue() {destroy();}

        bool init()
        {
                m_front = m_rear = new qnode<QElmType>;
                if (!m_front)
                        return false;
                m_front->next = 0;
                return true;
        }
        void destroy()
        {
                while (m_front)
                {
                        m_rear = m_front->next;
                        delete m_front;
                        m_front = m_rear;
                }
        }
        bool push(QElmType e)
        {
                struct qnode<QElmType> *p = new qnode<QElmType>;
                if (!p)
                        return false;
                p->next = 0;
                m_rear->next = p;
                m_rear->data = e;
                m_rear = p;
                return true;
        }
        bool pop(QElmType *e)
        {
                if (m_front == m_rear)
                        return false;


                struct qnode<QElmType> *p = m_front;
                *e = p->data;
                m_front = p->next;
                delete p;
                return true;
        }
private:
  struct qnode<QElmType> * volatile m_front, * volatile m_rear;
};


queue<int> g_q;


void *thread1(void * l)
{
        int i = 0;
        while (1)
        {
                g_q.push(i);
                i++;
                usleep(::rand()%1000);
        }
        return 0;
}
void *thread2(void * l)
{
        int i;
        while (1)
        {
                if (g_q.pop(&i))
                        std::cout << i << std::endl;
                //else
                        //std::cout << "can not pop" << std::endl;
                usleep(::rand()%1000);
        }
        return 0;
}


int main(int argc, char* argv[])
{
        pthread_t t1,t2;
        pthread_create(&t1, 0, thread1, 0);
        pthread_create(&t2, 0, thread2, 0);
        char ch;
        while (1)
        {
                std::cin >> ch;
                if (ch == 'q')
                        break;
        }
       return 0;
}

Answer 1

No. volatile does not guarantee multithread safety.

Note the title, and note the source:

Volatile: Almost Useless for Multi-Threaded Programming

There is a widespread notion that the keyword volatile is good for multi-threaded programming. I've seen interfaces with volatile qualifiers justified as "it might be used for multi-threaded programming". I thought was useful until the last few weeks, when it finally dawned on me (or if you prefer, got through my thick head) that volatile is almost useless for multi-threaded programming. I'll explain here why you should scrub most of it from your multi-threaded code.

...

Answer 2

volatile can work in limited circumstances, but not the way you're using it.

---

You've also got a bug or two and allocating a dummy node during init just complicates things:

bool
push(QElmType e)
{
    struct qnode <QElmType> *p = new qnode <QElmType>;

    if (!p)
        return false;

    p->next = 0;

    // BUG: _not_ thread safe because multiple things being updated
    m_rear->next = p;

    // BUG: you want to set p->data here
    m_rear->data = e;

    m_rear = p;

    return true;
}

bool
pop(QElmType *e)
{

    if (m_front == m_rear)
        return false;

    struct qnode <QElmType> *p = m_front;

    *e = p->data;
    m_front = p->next;
    delete p;

    return true;
}

---

Here's the cleaned up code using a lock. Note: After thinking about it, if you were trying to do a "ring queue" implementation, I [inadvertently] simplified that to a non-circular list. I was more concerned about the locking. Even with the original version, the lock is still required

bool
init()
{

    m_front = m_rear = nullptr;

    return true;
}

bool
push(QElmType e)
{
    struct qnode <QElmType> *p = new qnode <QElmType>;

    if (! p)
        return false;

    p->next = 0;
    p->data = e;

    // with the lock, now the _multiple_ can be updated _atomically_
    lock();

    if (m_front == nullptr)
        m_front = p;

    if (m_rear != nullptr)
        m_rear->next = p;

    m_rear = p;

    unlock();

    return true;
}

bool
pop(QElmType *e)
{
    bool valid;

    lock();

    struct qnode <QElmType> *p = m_front;

    valid = (p != nullptr);

    if (valid) {
        *e = p->data;

        m_front = p->next;
        if (p == m_rear)
            m_rear = m_front;

        delete p;
    }

    unlock();

    return valid;
}

---

A simple valid use of volatile would be:

volatile int stopflg;

void *
thread1(void *l)
{
    while (! stopflg) {
        // ...
    }

    return 0;
}

void *
thread2(void *l)
{
    while (! stopflg) {
        //...
    }

    return 0;
}

int
main(int argc, char *argv[])
{
    pthread_t t1,
     t2;

    pthread_create(&t1, 0, thread1, 0);
    pthread_create(&t2, 0, thread2, 0);

    char ch;

    while (1) {
        std::cin >> ch;
        if (ch == 'q') {
            stopflg = 1;
            break;
        }
    }

    return 0;
}

Answer 3

volatile is a compiler directive, stating that the variable can change at any time. It's to prevent the compiler from doing optimizations that would result in errors. Useful when writing C code for hardware that's memory mapped - I/O on the device can change the state of the variable.

It really has nothing to do with writing multi threaded coded.

Why is volatile needed in C?