2
lst = [1, 2, 3, 4, 5, 6, 7, 8, 9]

class Thread(QRunnable):
    def __init__(self):
        super(Thread, self).__init__()
        self.mutex = QMutex()

    def run(self):
        self.mutex.lock()
        lst.pop(0)
        print(str(lst))
        time.sleep(5)
        self.mutex.unlock()

The code above is an example of what I am trying to acheive, I have a list that is defined outside the class. I want to periodically pop the first element of the list. If I am running 5 threads I only want one thread to mutate the list at single time. Everytime I try this the 5 threads all try to pop the first element and dont wait as I want. When I recreate this in the native python threading library, it works as intended. What am I doing wrong here?

Larry
  • 117
  • 2
  • 13
  • 1
    It seems you're missing `super().__init__()` in the `__init__` of QRunnable. If it was just a typo for the question and it exists in your original code, then provide a [mre]. – musicamante Sep 12 '21 at 17:06
  • Any reason why this isn't a running example? – tdelaney Sep 12 '21 at 17:17
  • @musicamante Thank you, my bad, I forgot to include that. I have updated the answer. tdelaney's answer solved the issue. – Larry Sep 13 '21 at 00:59

1 Answers1

1

The problem is that you create a mutex per thread. A mutex lock only protects from threads using the same mutex. Since each thread is using its own private lock, its only protected from itself. Expanding on @eyllanesc answer, I've created a single mutex for all threads to use. The mutex should be considered associated with the data it protects.

import sys
import time

from PyQt6.QtCore import QCoreApplication, QMutex, QRunnable, QThreadPool, QTimer

# data with mutex access controls    
mutex = QMutex()
lst = [1, 2, 3, 4, 5, 6, 7, 8, 9]


class Thread(QRunnable):
    def run(self):
        #mutex = QMutex()    <-- don't create a private mutex
        mutex.lock()
        lst.pop(0)
        print(lst, time.time()-start, flush=True)
        time.sleep(5)
        mutex.unlock()

start = time.time()

def main():
    app = QCoreApplication(sys.argv)

    QTimer.singleShot(8 * 1000, QCoreApplication.quit)
    pool = QThreadPool()
    for i in range(5):
        runnable = Thread()
        pool.start(runnable)
    sys.exit(app.exec())


if __name__ == "__main__":
    main()
tdelaney
  • 73,364
  • 6
  • 83
  • 116
  • Technically, you can just use a single `Thread` instance, since QRunnable allows starting the `run` function multiple times, so the mutex can be an instance attribute. – musicamante Sep 12 '21 at 17:49
  • @musicamante - That could well be true. I'm not a pyqt expert and am depending on an existing example to demonstrate the problem. I think it would be good to have a better example than what I've given. It its something easy, you could perhaps make a new answer. – tdelaney Sep 12 '21 at 17:54
  • I don't think that adding another answer would add anything particolarly new to what you already did. Also, since the list is global, it makes sense to also have a global mutex (I think we all know the issues with global objects, though). Unfortunately, since the OP provided a very abstract example, it's difficult to provide a more appropriate answer: depending on the situation, it may have sense to have multiple `Thread` instances (for instance, provide an alternative argument for `pop`), and eventually share the mutex as a class attribute instead. – musicamante Sep 12 '21 at 18:05
  • @musicamante - your comment is probably good enough to let users know the code could be improved. I put the global mutex next to the list has a hint about what it controls. – tdelaney Sep 12 '21 at 18:11