AddressBook: thread-safety, fix use-after-free

src/libwalletqt/AddressBook.cpp

@@ -46,58 +46,85 @@ int AddressBook::errorCode() const
     return m_addressBookImpl->errorCode();
 }
 
-QList<Monero::AddressBookRow*> AddressBook::getAll(bool update) const
+void AddressBook::getAll()
 {
     qDebug(__FUNCTION__);
 
     emit refreshStarted();
 
-    if(update)
-        m_rows.clear();
+    {
+        QWriteLocker locker(&m_lock);
 
-    if (m_rows.empty()){
+        m_rows.clear();
         for (auto &abr: m_addressBookImpl->getAll()) {
             m_rows.append(abr);
         }
     }
 
     emit refreshFinished();
-    return m_rows;
-
 }
 
-Monero::AddressBookRow * AddressBook::getRow(int index) const
+bool AddressBook::getRow(int index, std::function<void (Monero::AddressBookRow &)> callback) const
 {
-    return m_rows.at(index);
+    QReadLocker locker(&m_lock);
+
+    if (index < 0 || index >= m_rows.size())
+    {
+        return false;
+    }
+
+    callback(*m_rows.value(index));
+    return true;
 }
 
-bool AddressBook::addRow(const QString &address, const QString &payment_id, const QString &description) const
+bool AddressBook::addRow(const QString &address, const QString &payment_id, const QString &description)
 {
     //  virtual bool addRow(const std::string &dst_addr , const std::string &payment_id, const std::string &description) = 0;
-    bool r = m_addressBookImpl->addRow(address.toStdString(), payment_id.toStdString(), description.toStdString());
+    bool result;
 
-    if(r)
-        getAll(true);
+    {
+        QWriteLocker locker(&m_lock);
 
-    return r;
+        result = m_addressBookImpl->addRow(address.toStdString(), payment_id.toStdString(), description.toStdString());
+    }
+
+    if (result)
+    {
+        getAll();
+    }
+
+    return result;
 }
 
-bool AddressBook::deleteRow(int rowId) const
+bool AddressBook::deleteRow(int rowId)
 {
-    bool r = m_addressBookImpl->deleteRow(rowId);
+    bool result;
+
+    {
+        QWriteLocker locker(&m_lock);
+
+        result = m_addressBookImpl->deleteRow(rowId);
+    }
 
     // Fetch new data from wallet2.
-    getAll(true);
+    if (result)
+    {
+        getAll();
+    }
 
-    return r;
+    return result;
 }
 
 quint64 AddressBook::count() const
 {
+    QReadLocker locker(&m_lock);
+
     return m_rows.size();
 }
 
 int AddressBook::lookupPaymentID(const QString &payment_id) const
 {
+    QReadLocker locker(&m_lock);
+
     return m_addressBookImpl->lookupPaymentID(payment_id.toStdString());
 }

src/libwalletqt/AddressBook.h

@@ -31,6 +31,7 @@
 
 #include <wallet/api/wallet2_api.h>
 #include <QObject>
+#include <QReadWriteLock>
 #include <QList>
 #include <QDateTime>
 
@@ -43,10 +44,9 @@ class AddressBook : public QObject
 {
     Q_OBJECT
 public:
-    Q_INVOKABLE QList<Monero::AddressBookRow*> getAll(bool update = false) const;
-    Q_INVOKABLE Monero::AddressBookRow * getRow(int index) const;
-    Q_INVOKABLE bool addRow(const QString &address, const QString &payment_id, const QString &description) const;
-    Q_INVOKABLE bool deleteRow(int rowId) const;
+    Q_INVOKABLE bool getRow(int index, std::function<void (Monero::AddressBookRow &)> callback) const;
+    Q_INVOKABLE bool addRow(const QString &address, const QString &payment_id, const QString &description);
+    Q_INVOKABLE bool deleteRow(int rowId);
     quint64 count() const;
     Q_INVOKABLE QString errorString() const;
     Q_INVOKABLE int errorCode() const;
@@ -61,6 +61,8 @@ public:
 
     Q_ENUM(ErrorCode);
 
+private:
+    void getAll();
 
 signals:
     void refreshStarted() const;
@@ -73,6 +75,7 @@ private:
     explicit AddressBook(Monero::AddressBook * abImpl, QObject *parent);
     friend class Wallet;
     Monero::AddressBook * m_addressBookImpl;
+    mutable QReadWriteLock m_lock;
     mutable QList<Monero::AddressBookRow*> m_rows;
 };