[Qt-interest] AmIRight: Can't Have Model and View on different Threads?

[Hostile Fork]

Hello all...!

I'm on day #2 of looking at Qt's Model/View architecture, and had a  
question that turned into an essay.  By the end of it, I figured I'd  
most likely answered what I started with... but I thought I'd run it  
by qt-interest (for criticisms if I'm wrong, and for Internet- 
Searching-Posterity if I'm right!)

The short version is that I don't think it's feasible for a Model to  
be modified on a non-GUI thread...regardless of whether the model's  
data has been protected with read/write locks.   If what I'm gathering  
is correct, then Qt should probably have an assert that a model and  
its view have the same thread affinity (it doesn't seem to do that now)

Here's what I think doesn't work:

	class NotReallyThreadSafeStandardItemModel :
		 public QStandardItemModel
	{
	private:
		QReadWriteLock dataLock;
	public:
		bool insertRows(int row, int count, const QModelIndex& parent =  
QModelIndex()) {
			dataLock.lockForWrite();
			QStandardItemModel::insertRows(row, count, parent);
			dataLock.unlock();
		}
		QVariant data(const QModelIndex& index, int role = Qt::DisplayRole)  
const {
			dataLock.lockForRead();
			QVariant result = QStandardItemModel::data(index, role);
			dataLock.unlock();
			return result;
		}
		...
	};

What I see as the problem is that the model/view updating strategy  
relies on signals and slots, and it's not expecting them to be queued.

For instance: the Gui View might see a model has 4 items, and set a  
variable ItemsDrawn to 4.  Then later it gets a notification saying  
"I'm adding one row", so it sets ItemsDrawn to 5 and tries to fetch  
the 5th element to draw.  But this could fail if that item has already  
been removed from the data by another thread.  This may have happened,  
but the "I'm deleting one row" notification is still pending in the  
Gui queue after the current "adding" notification we are processing...!

This would seem to suggest that it is not practical to have a model  
and view on separate threads.  As a thought-experiment, I've scribbled  
some rough code which tries for a "correct" 2-threaded  
StandardItemModel:

	class TwoThreadedStandardItemModel : public QStandardItemModel {
	private:
		QReadWriteLock dataLock;
		QMutex insertMutex;
		QWaitCondition insertDone;
		bool insertSuccess;
		...
	signals:
		bool insertRowsSignal(int row, int count, const QModelIndex & parent);
	protected slots:
		void insertRowsSlot(int row, int count, const QModelIndex & parent) {
			assert(currentThreadIsGui());
			assert(sender()->thread() == workerThread());
			dataLock.lockForWrite();
			bool success = QStandardItemModel::insertRows(row, count, parent);
			dataLock.unlock();

			insertMutex.lock();
			insertSuccess = success;
			insertDone.wakeOne();
			insertMutex.unlock();
		}
		...
	public:
		bool insertRow(int row, int count, const QModelIndex & parent =  
QModelIndex()) {
			bool success;
			if (currentThreadIsWorker()) {
				insertMutex.lock();
				emit insertRowsSignal(row, count, parent, false); // cross-thread,  
queued
				insertDone.wait(&insertMutex);
				success = insertSuccess;
				insertMutex.unlock();
			} else {
				dataLock.lockForWrite();
				success = QStandardItemModel::insertRows(row, count, parent);
				dataLock.unlock();
			}
			return success;
		}
		...
		QVariant data(const QModelIndex & index, int role = Qt::DisplayRole)  
const {
			// reading does not need to use a signal
			dataLock.lockForRead();
			QVariant result = QStandardItemModel::data(index, role);
			dataLock.unlock();
			return result;
		}
		...
		ThreadsafeStandardItemModel () {
			connect(this, SIGNAL(insertRowsSignal(int, int, const QModelIndex&,  
bool),
				this, SLOT(insertRowsSlot(int, int, const QModelIndex&, bool));
			...
		}
		...
	};

Despite the synchronization overhead, I imagine this approach could  
avoid crashes.  But if the View class you are using can do any  
autonomous modifications (like drag and drop rearrangement), then it  
could break the expectations of worker thread code, such as:

	model->insertRow(0);
		// Gui could modify model here!
	model->setData(model->index(0, 0), subject);
		// Gui could modify model here!
	model->setData(model->index(0, 1), sender);
		// Gui could modify model here!
	model->setData(model->index(0, 2), date);

Any of those opportunities for modification could delete or move the  
row we are working with.  A better strategy is for the worker to do  
updates via larger atomic asynchronous operations queued to the GUI  
thread.  Each should have no reliance on the prior to have finished to  
perform the next, e.g.

	public slots:
		void addMailMessage(const QString &subject, const QString &sender,  
const QDateTime &date)
		{
			assert(currentThreadIsGui()); // Modifications below will be atomic!
			model->insertRow(0);
			model->setData(model->index(0, 0), subject);
			model->setData(model->index(0, 1), sender);
			model->setData(model->index(0, 2), date);
		}

So... errr... AmiIRight? :)

Thanks!
	Brian

---
http://hostilefork.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.qt-project.org/pipermail/qt-interest-old/attachments/20090806/70fbf195/attachment.html