Add some basic read-write-locking to the previewDevices.

Remove comment about race conditions when setting the selected device. Turns
out this wasn't at all the case. It was a signal loop, instead.

svn path=/trunk/extragear/sysadmin/partitionmanager/; revision=1089499

src/core/devicescanner.cpp

@@ -90,4 +90,5 @@ void DeviceScanner::run()
 	}
 
 	operationStack().sortDevices();
+	emit devicesChanged();
 }

src/core/operationstack.cpp

@@ -38,14 +38,17 @@
 
 #include "util/globallog.h"
 
-
 #include <kdebug.h>
 #include <klocale.h>
 
+#include <QReadLocker>
+#include <QWriteLocker>
+
 /** Constructs a new OperationStack */
 OperationStack::OperationStack() :
 	m_Operations(),
-	m_PreviewDevices()
+	m_PreviewDevices(),
+	m_Lock(QReadWriteLock::Recursive)
 {
 }
 
@@ -420,6 +423,8 @@ void OperationStack::clearOperations()
 /** Clears the list of Devices. */
 void OperationStack::clearDevices()
 {
+	QWriteLocker lockDevices(&lock());
+
 	qDeleteAll(previewDevices());
 	previewDevices().clear();
 }
@@ -430,6 +435,8 @@ void OperationStack::clearDevices()
 */
 Device* OperationStack::findDeviceForPartition(const Partition* p)
 {
+	QReadLocker lockDevices(&lock());
+
 	foreach (Device* d, previewDevices())
 	{
 		if (d->partitionTable() == NULL)
@@ -456,6 +463,8 @@ void OperationStack::addDevice(Device* d)
 {
 	Q_ASSERT(d);
 
+	QWriteLocker lockDevices(&lock());
+
 	previewDevices().append(d);
 }
 
@@ -466,5 +475,7 @@ static bool deviceLessThan(const Device* d1, const Device* d2)
 
 void OperationStack::sortDevices()
 {
+	QWriteLocker lockDevices(&lock());
+
 	qSort(previewDevices().begin(), previewDevices().end(), deviceLessThan);
 }

src/core/operationstack.h

@@ -22,6 +22,8 @@
 #define OPERATIONSTACK__H
 
 #include <QList>
+#include <QReadWriteLock>
+
 #include <qglobal.h>
 
 class Device;
@@ -64,6 +66,8 @@ class OperationStack
 
 		Device* findDeviceForPartition(const Partition* p);
 
+		QReadWriteLock& lock() { return m_Lock; }
+
 	protected:
 		void clearDevices();
 		void addDevice(Device* d);
@@ -78,6 +82,7 @@ class OperationStack
 	private:
 		Operations m_Operations;
 		mutable Devices m_PreviewDevices;
+		QReadWriteLock m_Lock;
 };
 
 #endif

src/gui/listdevices.cpp

@@ -26,6 +26,7 @@
 
 #include <kmenu.h>
 #include <kactioncollection.h>
+#include <kdebug.h>
 
 class ListDeviceWidgetItem : public QListWidgetItem
 {

src/gui/mainwindow.cpp

@@ -34,6 +34,7 @@
 #include <kstandardguiitem.h>
 
 #include <QCloseEvent>
+#include <QReadLocker>
 
 #include <config.h>
 
@@ -176,6 +177,8 @@ void MainWindow::on_m_PartitionManagerWidget_operationsChanged()
 
 void MainWindow::on_m_PartitionManagerWidget_devicesChanged()
 {
+	QReadLocker lockDevices(&pmWidget().operationStack().lock());
+
 	listDevices().updateDevices(pmWidget().previewDevices(), pmWidget().selectedDevice());
 
 	if (pmWidget().selectedDevice())

src/gui/partitionmanagerwidget.cpp

@@ -66,6 +66,7 @@
 
 #include <QCursor>
 #include <QPointer>
+#include <QReadLocker>
 
 #include <config.h>
 
@@ -325,15 +326,10 @@ void PartitionManagerWidget::onScanDevicesProgressChanged(const QString& device_
 
 void PartitionManagerWidget::onScanDevicesFinished()
 {
+	QReadLocker lockDevices(&operationStack().lock());
+
 	if (!operationStack().previewDevices().isEmpty())
-	{
 		setSelectedDevice(operationStack().previewDevices()[0]);
-		// FIXME: Normally this should be emitted in setSelectedDevice(), but if we do that
-		// now we get all kinds of terrible races and crashes during rescan (because DeviceScanner
-		// clears the devices in the operationstack while ListDevices is just iterating them).
-		// Once that is fixed, remove the emit here.
-		emit devicesChanged();
-	}
 
 	updatePartitions();
 
@@ -398,6 +394,8 @@ void PartitionManagerWidget::clearSelectedPartition()
 
 void PartitionManagerWidget::setSelectedDevice(const QString& device_node)
 {
+	QReadLocker lockDevices(&operationStack().lock());
+
 	foreach(Device* d, operationStack().previewDevices())
 		if (d->deviceNode() == device_node)
 		{
@@ -410,9 +408,10 @@ void PartitionManagerWidget::setSelectedDevice(const QString& device_node)
 
 void PartitionManagerWidget::setSelectedDevice(Device* d)
 {
+	// NOTE: we cannot emit devicesChanged() here because it will end up calling
+	// ListDevices::updateDevices() which in turn will modify the QListWidget, which
+	// will then emit itemSelectionChanged() which will in the end lead us back here.
 	m_SelectedDevice = d;
-	// FIXME: We should emit devicesChanged() here, but if we do that we get terrible
-	// races. See onScanDevicesFinished()
 	clearSelectedPartition();
 }
 

src/gui/partitionmanagerwidget.h

@@ -80,6 +80,9 @@ class LIBPARTITIONMANAGERPRIVATE_EXPORT PartitionManagerWidget : public QWidget,
 		const OperationStack::Devices& previewDevices() const { return operationStack().previewDevices(); }
 		const OperationStack::Operations& operations() const { return operationStack().operations(); }
 
+		OperationStack& operationStack() { return m_OperationStack; }
+		const OperationStack& operationStack() const { return m_OperationStack; }
+
 		void updatePartitions();
 
 		Partition* clipboardPartition() { return m_ClipboardPartition; }
@@ -111,9 +114,6 @@ class LIBPARTITIONMANAGERPRIVATE_EXPORT PartitionManagerWidget : public QWidget,
 		OperationRunner& operationRunner() { return m_OperationRunner; }
 		const OperationRunner& operationRunner() const { return m_OperationRunner; }
 
-		OperationStack& operationStack() { return m_OperationStack; }
-		const OperationStack& operationStack() const { return m_OperationStack; }
-
 		DeviceScanner& deviceScanner() { return m_DeviceScanner; }
 		const DeviceScanner& deviceScanner() const { return m_DeviceScanner; }
 

src/kcm/partitionmanagerkcm.cpp

@@ -34,6 +34,7 @@
 #include <kcmultidialog.h>
 
 #include <QTimer>
+#include <QReadLocker>
 
 K_PLUGIN_FACTORY(
 		PartitionManagerKCMFactory,
@@ -116,6 +117,8 @@ void PartitionManagerKCM::on_m_PartitionManagerWidget_operationsChanged()
 
 void PartitionManagerKCM::on_m_PartitionManagerWidget_devicesChanged()
 {
+	QReadLocker lockDevices(&pmWidget().operationStack().lock());
+
 	listDevices().updateDevices(pmWidget().previewDevices(), pmWidget().selectedDevice());
 }