Access to TableModel methods from other thread

[ichmeins]

I am skimming through my code and noticed a circumstance where I'm not sure whether it is potentially dangerous.

I have a QTableView to visualize a download queue. It uses a custom QAbstractTableModel subclass (using pyqt5 here) :

class DownloadQueueTableModel(QAbstractTableModel):
 
    downloadQueue = deque()    
    queueLock = Lock()
 
    def addToQueue(self, item):
 
        row = self.queueLength()
        self.beginInsertRows(QModelIndex(), row, row)
 
        with self.queueLock:
            self.downloadQueue.append(item)
 
        self.endInsertRows()
 
    def getNextItem(self):
        if self.queueLength() == 0:
            return None
        self.beginRemoveRows(QModelIndex(), 0, 0)
        with self.queueLock:
            item =  self.downloadQueue.popleft()
        self.endRemoveRows()
        return item

class DownloadQueueTableModel(QAbstractTableModel):

    downloadQueue = deque()    
    queueLock = Lock()

    def addToQueue(self, item):
        
        row = self.queueLength()
        self.beginInsertRows(QModelIndex(), row, row)
        
        with self.queueLock:
            self.downloadQueue.append(item)
        
        self.endInsertRows()

    def getNextItem(self):
        if self.queueLength() == 0:
            return None
        self.beginRemoveRows(QModelIndex(), 0, 0)
        with self.queueLock:
            item =  self.downloadQueue.popleft()
        self.endRemoveRows()
        return item

To copy to clipboard, switch view to plain text mode 

My table model uses an internal queue to save the data. While the addToQueue method is called as a reaction to a button click - and therefore in the context of the UI thread - the getNextItem method is called from a worker thread, which downloads the items one after the other.
The access to the queue data structure itself is protected through a lock and therefore thread safe, the only question that remains:
Is a call to beginRemoveRows and endRemoveRows from another thread, which is not the UI thread, safe?

The application is running correctly and I have not noticed any problems so far, but we all now that errors caused by racing conditions are very random and hard to find later on.

[anda_skoa]

I assume you also have the lock in rowCount() and data(), but in general that is not safe.

What you could consider instead is the thread emitting a signal when it is ready for a new item and connecting that to a model slot using Qt::QueuedConnection as the connection type.
That slot is then called by the main thread which could then dequeue the item and send it to the worker thread (either again via a QueuedConnection signal or by calling a method that internally uses a lock to protect against concurrent access)

Cheers,
_

[ichmeins]

Ok, thanks for the clarification.
I'll let the worker thread just read the first position in the queue and signal the UI thread to remove the item. That would only require a one way communication between threads.

[anda_skoa]

True, but that still requires your to lock the queue and the worker could work on the same item multiple times.

Cheers,
_