CODEWITHSUNDEEP
pythonpyqtmutex
Gio
Gio

Reputation: 3340

Is a mutex really necessary in this piece of code?

The code below shows a class which I use for PyQt threading functionality. In my main program I instantiate and start this thread (i.e. I use PyQt's moveToThread, and countdown is the run method.). While the thread is running I frequently call the reset method. I'm not sure if the mutex I've implemented is actually necessary, I hope somebody can clear this up for me. I've executed a quick test in which I've commented out the mutex, this gave me no problems, even when I comment out the time.sleep(1) and call the reset method while the thread is running without any delay. However I wan't to be 100% secure, hence the reason for my question here.

import time
from PyQt4.QtCore import *
from threading import  Lock

class Countdown(QObject):
    finished = pyqtSignal()

    def __init__(self, countdownInSec=30):
        super(Countdown, self).__init__()
        self.COUNTDOWN_IN_SEC = countdownInSec
        self._countdownInSec = self.COUNTDOWN_IN_SEC
        self._mutex = Lock()

    @pyqtSlot()
    def countdown(self):
        while self._countdownInSec > 0:
            print(self._countdownInSec)
            self._mutex.acquire()
            try:
                self._countdownInSec -= 1
            finally:
                self._mutex.release()
            time.sleep(1)
        self.finished.emit()

    def increment(self, seconds):
        self._mutex.acquire()
        try:
            self._countdownInSec += seconds
        finally:
            self._mutex.release()

    def reset(self):
        self._mutex.acquire()
        try:
            self._countdownInSec = self.COUNTDOWN_IN_SEC
        finally:
            self._mutex.release()

extraction of main (only the part that is relevant for this question)

    fpIntervUpdCountdownReset = pyqtSignal()                                             

    def __init__(self):
        self.initFlightPathIntervUpdater()

    def initFlightPathIntervUpdater(self):             
       self.fpIntervUpdCountdownThr = QThread()                                         
       self.fpIntervUpdCountdown = countdown.Countdown()
       self.fpIntervUpdCountdown.moveToThread(self.fpIntervUpdCountdownThr)      
       self.fpIntervUpdCountdown.finished.connect(self.fpIntervUpdCountdownThr.quit)    
       self.fpIntervUpdCountdown.finished.connect(self.flightPathIntervUpdate)                  
       self.fpIntervUpdCountdownThr.started.connect(self.fpIntervUpdCountdown.countdown)

   def flightPathIntervUpdateReq(self):                                                 
       if self.fpIntervUpdCountdownThr.isRunning():                                     
           self.fpIntervUpdCountdown.reset()                                            
       else:                                                                            
           print 'start'                                                                
           self.fpIntervUpdCountdownThr.start()                                         

   @pyqtSlot()                                                                          
   def flightPathIntervUpdate(self):                                                    
       print "perform flightPathIntervUpdate"

Upvotes: 1

Views: 169

Answers (1)

swenzel
swenzel

Reputation: 7173

I would leave it where it is.
If you're unlucky your reset/increment is performed by one thread in the small time window just after the countdown thread reads the counter and before it writes back the result of its decrement. It would then overwrite the updated value without even seeing it.
That time window is incredibly small so it might be very unlikely to happen, but it is possible.

By the way the preferred way to use locks is:

def increment(self, seconds):
    with self._mutex:
        self._countdownInSec += seconds

Upvotes: 1

Related Questions