Handle cross-thread synchronization when destructing a peripheral
MODIFY
src/io/iobase.cpp
src/io/iobase.cpp
#include "iobase.h"
+#include <condition_variable>
+
namespace Ripes {
std::map<std::type_index, int> IOBase::s_peripheralCount;
IOBase::IOBase(QWidget* parent) : QWidget(parent) {
- registerPeripheral(this);
-
+ incrementPeriphCounter(this);
connect(this, &IOBase::scheduleUpdate, this, QOverload<>::of(&QWidget::update));
}
@@ -23,4 +24,18 @@ bool IOBase::setParameter(unsigned ID, const QVariant& value) {
return true;
}
+void IOBase::unregister() {
+ std::atomic<bool> sync;
+ std::condition_variable cv;
+ std::mutex cv_m;
+ std::unique_lock<std::mutex> lock(cv_m);
+
+ // Signal to IO Manager that we're about to delete this peripheral, allowing it to unregister this.
+ emit aboutToDelete(sync);
+ cv.wait(lock, [&sync] { return sync == 1; });
+ m_didUnregister = true;
+
+ // Continue destruction process...
+}
+
} // namespace Ripes

MODIFY
src/io/iobase.h
src/io/iobase.h
@@ -38,7 +38,10 @@ class IOBase : public QWidget {
public:
IOBase(QWidget* parent);
- virtual ~IOBase() { deregisterPeripheral(this); };
+ virtual ~IOBase() {
+ assert(m_didUnregister && "IO peripherals must call unregister() in their destructor!");
+ decrementPeriphCounter(this);
+ };
const std::map<unsigned, IOParam>& parameters() const { return m_parameters; }
virtual QString description() const = 0;
@@ -83,20 +86,43 @@ signals:
*/
void scheduleUpdate();
+ /**
+ * @brief aboutToDelete
+ * Signal emitted when this IO peripheral is about to be destroyed. @param ok should be set to 'true' once the
+ * surrounding environment acknowledges (ie. after peripheral-dependees have been notified).
+ */
+ void aboutToDelete(std::atomic<bool>& ok);
+
protected:
+ /**
+ * @brief unregister
+ * Unregisters the IO peripheral with the remaining compute system in a thread-safe manner. This function is key in
+ * avoiding race conditions from a processor model executing on another thread, trying to access this peripherals
+ * i/o functions while it is being deleted. This function !must! be called from an inheriting IO peripherals'
+ * constructor.
+ */
+ void unregister();
+
virtual void parameterChanged(unsigned ID) = 0;
std::map<unsigned, IOParam> m_parameters;
static std::map<std::type_index, int> s_peripheralCount;
- static void registerPeripheral(const IOBase* p) {
+ static void incrementPeriphCounter(const IOBase* p) {
s_peripheralCount[typeid(p)]++;
return;
}
- static void deregisterPeripheral(const IOBase* p) {
+ static void decrementPeriphCounter(const IOBase* p) {
Q_ASSERT(s_peripheralCount.at(typeid(p)) > 0);
s_peripheralCount[typeid(p)]--;
}
+
+private:
+ /**
+ * @brief m_didUnregister
+ * Debug variable to ensure that inheriting class called unregister() on delete.
+ */
+ bool m_didUnregister = false;
};
} // namespace Ripes

MODIFY
src/io/ioledmatrix.h
src/io/ioledmatrix.h
@@ -15,7 +15,7 @@ class IOLedMatrix : public IOBase {
public:
IOLedMatrix(QWidget* parent);
- ~IOLedMatrix(){};
+ ~IOLedMatrix() { unregister(); };
virtual uint32_t size() const override;
virtual QString description() const override;

MODIFY
src/io/iomanager.cpp
src/io/iomanager.cpp
@@ -56,6 +56,7 @@ IOBase* IOManager::createPeripheral(IOType type) {
auto* peripheral = IOFactories.at(type)(nullptr);
connect(peripheral, &IOBase::sizeChanged, [=] { this->peripheralSizeChanged(peripheral); });
+ connect(peripheral, &IOBase::aboutToDelete, [=](std::atomic<bool>& ok) { this->removePeripheral(peripheral, ok); });
// Assign base address to peripheral
const uint32_t base = nextPeripheralAddress();
@@ -66,12 +67,16 @@ IOBase* IOManager::createPeripheral(IOType type) {
return peripheral;
}
-void IOManager::removePeripheral(IOBase* peripheral) {
+void IOManager::removePeripheral(IOBase* peripheral, std::atomic<bool>& ok) {
auto periphit = m_peripherals.find(peripheral);
Q_ASSERT(periphit != m_peripherals.end());
unregisterPeripheralWithProcessor(peripheral);
m_peripherals.erase(periphit);
refreshMemoryMap();
+
+ emit peripheralRemoved(peripheral);
+
+ ok = true;
}
void IOManager::refreshAllPeriphsToProcessor() {

MODIFY
src/io/iomanager.h
src/io/iomanager.h
@@ -20,11 +20,12 @@ class IOManager : public QObject {
public:
IOManager();
IOBase* createPeripheral(IOType type);
- void removePeripheral(IOBase* peripheral);
+ void removePeripheral(IOBase* peripheral, std::atomic<bool>& ok);
const MemoryMap& memoryMap() const { return m_memoryMap; }
signals:
void memoryMapChanged();
+ void peripheralRemoved(QObject* peripheral);
private:
void refreshMemoryMap();

MODIFY
src/io/ioswitches.h
src/io/ioswitches.h
@@ -61,7 +61,7 @@ class IOSwitches : public IOBase {
public:
IOSwitches(QWidget* parent);
- ~IOSwitches(){};
+ ~IOSwitches() { unregister(); };
virtual uint32_t size() const override { return 4; }
virtual QString description() const override;

MODIFY
src/iotab.cpp
src/iotab.cpp
@@ -61,6 +61,8 @@ IOTab::IOTab(QToolBar* toolbar, QWidget* parent) : RipesTab(toolbar, parent), m_
}
});
+ connect(&m_iomanager, &IOManager::peripheralRemoved, this, &IOTab::removePeripheral);
+
connect(m_ui->peripheralsTab, &QTabWidget::currentChanged, this, &IOTab::setPeripheralMDIWindowActive);
m_ui->memoryMapView->setModel(new MemoryMapModel(&m_iomanager, this));
m_ui->memoryMapView->horizontalHeader()->setSectionResizeMode(MemoryMapModel::Name, QHeaderView::ResizeToContents);
@@ -87,14 +89,9 @@ void IOTab::createPeripheral(IOType type) {
mw->addDockWidget(Qt::TopDockWidgetArea, dw);
auto* mdiw = m_ui->mdiArea->addSubWindow(mw);
mdiw->setWindowTitle(IOTypeTitles.at(type));
+ peripheral->setFocus();
m_subWindows[peripheralTab] = mdiw;
-
- // Signals
-
- // Connect to the destruction of the peripheral to trigger required changes on peripheral deletion. The peripheral
- // is implicitly destructed when the MDIWindow is closed.
- connect(peripheral, &IOBase::destroyed, this, &IOTab::removePeripheral);
}
void IOTab::setPeripheralTabActive(IOBase* peripheral) {
@@ -119,11 +116,6 @@ void IOTab::setPeripheralMDIWindowActive(int tabIndex) {
}
void IOTab::removePeripheral(QObject* peripheral) {
- // This might be a little ugly. We have to static cast to an IOBase, given that, when QObject::destroyed is emitted,
- // the entire inheriting class hierarchy has already been destroyed (thus we cannot dynamic_cast to an IOBase).
- // However, the base pointer stays the same...
- m_iomanager.removePeripheral(static_cast<IOBase*>(peripheral));
-
auto* tab = m_ioTabs.at(peripheral);
Q_ASSERT(m_subWindows.count(tab) != 0);
m_subWindows.erase(tab);

MODIFY
src/iotab.h
src/iotab.h
@@ -27,9 +27,9 @@ class IOTab : public RipesTab {
public:
IOTab(QToolBar* toolbar, QWidget* parent = nullptr);
~IOTab() override;
+ void removePeripheral(QObject* peripheral);
private slots:
- void removePeripheral(QObject* peripheral);
/** The following to slots ensures synchronization between the current MDI window and peripheral tab */
void setPeripheralTabActive(IOBase* peripheral);

Handle cross-thread synchronization when destructing a peripheral