Producer–consumer multithread QQueue + QSemaphore

Drazz

Hello,

I've got two classes in different threads, consumer and producer that share one QQueue and one QSemaphore.

Producer

void Producer::run() {
    while(1) {
        if (dataReady) {
            queue.enqueue(data);
            semaphore.release();
        }
    }  
}

Consumer

void Consumer::run() {

    while(1) {
        if (semaphore.acquire(1)) {
            auto data = queue.dequeue();
            doSomething();
        }
    }
}

I'm wondering if my queue data is thread safe or if I should subclass QQueue and use QSemaphore and QMutex to protect the shared resoruce because (https://doc.qt.io/qt-5/qtcore-threads-semaphores-example.html) uses two QSemaphore.

Do I have to lock a mutex before enqueue/dequeue data and unlock it after? I yes, I don't understand the advantages of using QSemaphore here.

kshegunov

This is asked often, look here:

https://forum.qt.io/topic/75880/proper-way-to-manage-information-between-threads
https://forum.qt.io/topic/90127/blocking-queue-between-qthreads
https://forum.qt.io/topic/116093/queue-enque-and-deque-thread-safe

Christian Ehrlicher

Since the documentation of QQueue does not mention that it's thread-safe you must assume that it isn't thread-safe.

kshegunov

This is asked often, look here:

https://forum.qt.io/topic/75880/proper-way-to-manage-information-between-threads
https://forum.qt.io/topic/90127/blocking-queue-between-qthreads
https://forum.qt.io/topic/116093/queue-enque-and-deque-thread-safe

Drazz

Thanks for the info. I was able to find my answer on this thread:

@kshegunov said in Proper way to Manage Information Between Threads:

@VRonin said in Proper way to Manage Information Between Threads:

The semaphore is redundant here.

No, it is not!!!

@darkp03 said in Proper way to Manage Information Between Threads:

I was quite disappointed to know that reimplementing the run method isnt the correct way, but everywhere on internet says that.

Don't be, the internet is wrong. It's a valid thing to do if you don't need to use any of the signal-slot machinery. However your queue implementation looks a bit off. Consider this one:
Producer(s):
QByteArray data; //< Data to put in the queue

inputNeeds.acquire();	// First wait for more data to be consumed if needed (do not overfill the queue).

QMutexLocker lock(&queueMutex);
queue.enqueue(data);
queueAvailability.release();
Consumer(s):
queueAvailability.acquire();
QMutexLocker lock(&queueMutex);
if (queue.size() <= 0)
    return; //< So graceful exit is possible

QByteArray data = queue.dequeue();
inputNeeds.release();
Where variables are:
QSemaphore queueAvailability, inputNeeds; //< inputNeeds is initialized to some number (e.g. 10000) in the beginning - that's the maximum number of items the queue will hold so it doesn't overflow if the consumer(s) lag behind
QMutex queueMutex;
QQueue<QByteArray> queue;

So mutex and sempahore makes sense