Qt: do I need a mutex if there is only one writer thread?

Question

configuration:debug result: program exit with code 0.

configuration:release result: main thread infinite loops,wont jump out of the loop(t.n==0 is true).

how can I get out of the loop ?

Only one writer thread,so I didnot use any mutex.

qt5.13 vs2017

main.cpp:

//#include"QtTestProg.h"
#include<QApplication>
#include<QMessageBox>
#include<QThread>
class MyThread :public QThread
{
    Q_OBJECT
public:
    int n = 0, m = 1;
    void run()override;
};
void MyThread::run()
{
    for (; m;) {
        QThread::msleep(3000);
        n = 1;
    }
}
int main(int argc, char*argv[])
{
    QApplication a(argc, argv);
    MyThread t;
    t.start();
    for (; 1;)
    {
        if (t.n != 0)
        {
            break;
        }
    }
    t.m = 0;
    t.quit();
    t.wait();
    return 0;
}
#include"main.moc"

Your QThread is reading the value of `m` and the `main()` thread is writing the value of `m`, which means that access to `m` must be synchronized via a mutex (or alternatively `m` must be changed to an atomic type) otherwise you're invoking undefined behavior. (same problem with `n` except that the roles of the two threads are reversed) — Jeremy Friesner, Aug 25 '21 at 05:26

selbie · Answer 1 · 2021-08-25T04:26:22.710

2

This loop:

    for (; 1;)
    {
        if (t.n != 0)
        {
            break;
        }
    }

Technically, the above code could literally run forever even if the compiler sees that t.n is not getting modified. It could, theoretically, cache the value in a register or optimize it out. There's also the cache-coherency issue associated to doing lockless programming.

Three options:

Use std::atomic to have a fast lock-like and thread safe semantics on a single variable.
Just use a std::mutex and std::lock around the assignment and evaluation of t.n.
Use a condition variable such that the main thread waits for n to change until the running thread signals that it has changed.

edited Aug 25 '21 at 04:26

answered Aug 25 '21 at 04:19

selbie

100,020
15
103
173

: i dont understand : why ' the above code could literally run forever'? – kenash0625 Aug 26 '21 at 06:35
1

@kenash0625 - Here's a simple example validated by assembly: https://www.godbolt.org/z/adnKbzE6Y The compiler in this case will stuff the value of `n` into the `eax` register and just test if it's zero. If it is zero, it just does a jump back to the `test` line to continue evaluating the eax register. So even if `n` is changed by another thread, the loop will continue forever. – selbie Aug 28 '21 at 02:33
thanks. i switch between -O1 -O2 -O0 , find that the compiler did a lot of things. i should review assembly language now – kenash0625 Aug 30 '21 at 09:59

danadam · Accepted Answer · 2021-08-25T05:10:03.783

Only one writer thread,so I didnot use any mutex.

That's undefined behavior. If you read and write the same memory location from different threads, you need synchronization. From https://en.cppreference.com/w/cpp/language/memory_model

When an evaluation of an expression writes to a memory location and another evaluation reads or modifies the same memory location, the expressions are said to conflict. A program that has two conflicting evaluations has a data race unless

both evaluations execute on the same thread or in the same signal handler, or

both conflicting evaluations are atomic operations (see std::atomic), or

one of the conflicting evaluations happens-before another (see std::memory_order)

If a data race occurs, the behavior of the program is undefined.

You either need a mutex guarding both reads and writes or use std::atomic_int/QAtomicInt instead of plain int for m and n.

As for the consequences of the undefined behavior, here (godbolt) you can see that gcc at O2 level compiles out the loop in the main() entirely unless you change TYPE from int to std::atomic_int at the top.

It does not seem to eliminate loop in any case in godbolt, as far as I see. Are you sure? — Özgür Murat Sağdıçoğlu, Aug 25 '21 at 07:35
@Ozgur That's how I interpret no highlighted background. [Here is a screenshot](https://i.stack.imgur.com/WWc10.png) with `printf()` added before and after the loop, where it is visible better. You can also run it and see that there is no delay between messages unless you change the `TYPE` to `std::atomic_int`. If you run it inside godbolt though, you will probably have to add current time to those messages to see a difference. — danadam, Aug 25 '21 at 14:20

score 1 · Answer 3 · answered Aug 25 '21 at 11:03

1

You need to use QReadWriteLock. This class is designed for your cases, when you need several readers not to block each other.

answered Aug 25 '21 at 11:03

Dmitry Sazonov

8,801
1
35
61

Qt: do I need a mutex if there is only one writer thread?

3 Answers3