Daemon and Wallet RPC improvements and fixes

src/cpp/common/py_monero_common.cpp

@@ -1,6 +1,61 @@
 #include "py_monero_common.h"
 #include "utils/monero_utils.h"
 
+PyThreadPoller::~PyThreadPoller() {
+  set_is_polling(false);
+}
+
+void PyThreadPoller::init_common(const std::string& name) {
+  m_name = name;
+  m_is_polling = false;
+  m_poll_period_ms = 20000;
+  m_poll_loop_running = false;
+}
+
+void PyThreadPoller::set_is_polling(bool is_polling) {
+  if (is_polling == m_is_polling) return;
+  m_is_polling = is_polling;
+
+  if (m_is_polling) {
+    run_poll_loop();
+  } else {
+    if (m_poll_loop_running) {
+      m_poll_cv.notify_one();
+      std::this_thread::sleep_for(std::chrono::milliseconds(1));  // TODO: in emscripten, m_sync_cv.notify_one() returns without waiting, so sleep; bug in emscripten upstream llvm?
+      m_thread.join();
+    }
+  }
+}
+
+void PyThreadPoller::set_period_in_ms(uint64_t period_ms) {
+  m_poll_period_ms = period_ms;
+}
+
+void PyThreadPoller::run_poll_loop() {
+  if (m_poll_loop_running) return; // only run one loop at a time
+  m_poll_loop_running = true;
+
+  // start pool loop thread
+  // TODO: use global threadpool, background sync wasm wallet in c++ thread
+  m_thread = boost::thread([this]() {
+
+    // poll while enabled
+    while (m_is_polling) {
+      try { poll(); }
+      catch (const std::exception& e) { std::cout << m_name << " failed to background poll: " << e.what() << std::endl; }
+      catch (...) { std::cout << m_name << " failed to background poll" << std::endl; }
+
+      // only wait if polling still enabled
+      if (m_is_polling) {
+        boost::mutex::scoped_lock lock(m_polling_mutex);
+        boost::posix_time::milliseconds wait_for_ms(m_poll_period_ms.load());
+        m_poll_cv.timed_wait(lock, wait_for_ms);
+      }
+    }
+
+    m_poll_loop_running = false;
+  });
+}
 
 py::object PyGenUtils::convert_value(const std::string& val) {
   if (val == "true") return py::bool_(true);

src/cpp/common/py_monero_common.h

@@ -43,6 +43,30 @@ namespace pybind11 { namespace detail {
 
 }}
 
+class PyThreadPoller {
+public:
+
+  ~PyThreadPoller();
+
+  bool is_polling() const { return m_is_polling; }
+  void set_is_polling(bool is_polling);
+  void set_period_in_ms(uint64_t period_ms);
+  virtual void poll() = 0;
+
+protected:
+  std::string m_name;
+  boost::recursive_mutex m_mutex;
+  boost::mutex m_polling_mutex;
+  boost::thread m_thread;
+  std::atomic<bool> m_is_polling;
+  std::atomic<bool> m_poll_loop_running;
+  std::atomic<uint64_t> m_poll_period_ms;
+  boost::condition_variable m_poll_cv;
+
+  void init_common(const std::string& name);
+  void run_poll_loop();
+};
+
 class PySerializableStruct : public monero::serializable_struct {
 public:
   using serializable_struct::serializable_struct;

src/cpp/daemon/py_monero_daemon.h

@@ -141,8 +141,7 @@ class PyMoneroDaemon {
   virtual void submit_blocks(const std::vector<std::string>& block_blobs) { throw std::runtime_error("PyMoneroDaemon: not supported"); }
   virtual std::shared_ptr<PyMoneroPruneResult> prune_blockchain(bool check) { throw std::runtime_error("PyMoneroDaemon: not supported"); }
   virtual std::shared_ptr<PyMoneroDaemonUpdateCheckResult> check_for_update() { throw std::runtime_error("PyMoneroDaemon: not supported"); }
-  virtual std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> download_update() { throw std::runtime_error("PyMoneroDaemon: not supported"); }
-  virtual std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> download_update(const std::string& path) { throw std::runtime_error("PyMoneroDaemon: not supported"); }
+  virtual std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> download_update(const std::string& path = "") { throw std::runtime_error("PyMoneroDaemon: not supported"); }
   virtual void stop() { throw std::runtime_error("PyMoneroDaemon: not supported"); }
   virtual std::shared_ptr<monero::monero_block_header> wait_for_next_block_header() { throw std::runtime_error("PyMoneroDaemon: not supported"); }
 };

src/cpp/daemon/py_monero_daemon_rpc.cpp

@@ -4,38 +4,10 @@
 static const uint64_t MAX_REQ_SIZE = 3000000;
 static const uint64_t NUM_HEADERS_PER_REQ = 750;
 
-PyMoneroDaemonPoller::~PyMoneroDaemonPoller() {
-  set_is_polling(false);
-}
-
-PyMoneroDaemonPoller::PyMoneroDaemonPoller(PyMoneroDaemon* daemon, uint64_t poll_period_ms): m_poll_period_ms(poll_period_ms), m_is_polling(false) {
+PyMoneroDaemonPoller::PyMoneroDaemonPoller(PyMoneroDaemon* daemon, uint64_t poll_period_ms) {
   m_daemon = daemon;
-}
-
-void PyMoneroDaemonPoller::set_is_polling(bool is_polling) {
-  if (is_polling == m_is_polling) return;
-  m_is_polling = is_polling;
-
-  if (m_is_polling) {
-    m_thread = std::thread([this]() {
-      loop();
-    });
-    m_thread.detach();
-  } else {
-    if (m_thread.joinable()) m_thread.join();
-  }
-}
-
-void PyMoneroDaemonPoller::loop() {
-  while (m_is_polling) {
-    try {
-      poll();
-    } catch (const std::exception& e) {
-      std::cout << "ERROR " << e.what() << std::endl;
-    }
-
-    std::this_thread::sleep_for(std::chrono::milliseconds(m_poll_period_ms));
-  }
+  init_common("monero_daemon_rpc");
+  m_poll_period_ms = poll_period_ms;
 }
 
 void PyMoneroDaemonPoller::poll() {
@@ -73,6 +45,11 @@ PyMoneroDaemonRpc::PyMoneroDaemonRpc(const std::string& uri, const std::string&
   if (!uri.empty()) m_rpc->check_connection();
 }
 
+std::vector<std::shared_ptr<PyMoneroDaemonListener>> PyMoneroDaemonRpc::get_listeners() {
+  boost::lock_guard<boost::recursive_mutex> lock(m_listeners_mutex);
+  return m_listeners;
+}
+
 void PyMoneroDaemonRpc::add_listener(const std::shared_ptr<PyMoneroDaemonListener> &listener) {
   boost::lock_guard<boost::recursive_mutex> lock(m_listeners_mutex);
   m_listeners.push_back(listener);
@@ -777,17 +754,6 @@ std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> PyMoneroDaemonRpc::download_
   return result;
 }
 
-std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> PyMoneroDaemonRpc::download_update() {
-  auto params = std::make_shared<PyMoneroDownloadUpdateParams>();
-  PyMoneroPathRequest request("update", params);
-  auto response = m_rpc->send_path_request(request);
-  check_response_status(response);
-  auto result = std::make_shared<PyMoneroDaemonUpdateDownloadResult>();
-  auto res = response->m_response.get();
-  PyMoneroDaemonUpdateDownloadResult::from_property_tree(res, result);
-  return result;
-}
-
 void PyMoneroDaemonRpc::stop() {
   PyMoneroPathRequest request("stop_daemon");
   std::shared_ptr<PyMoneroPathResponse> response = m_rpc->send_path_request(request);
@@ -837,6 +803,7 @@ std::shared_ptr<PyMoneroBandwithLimits> PyMoneroDaemonRpc::set_bandwidth_limits(
 }
 
 void PyMoneroDaemonRpc::refresh_listening() {
+  boost::lock_guard<boost::recursive_mutex> lock(m_listeners_mutex);
   if (!m_poller && m_listeners.size() > 0) {
     m_poller = std::make_shared<PyMoneroDaemonPoller>(this);
   }
@@ -847,9 +814,10 @@ void PyMoneroDaemonRpc::check_response_status(const boost::property_tree::ptree&
   for (boost::property_tree::ptree::const_iterator it = node.begin(); it != node.end(); ++it) {
     std::string key = it->first;
     if (key == std::string("status")) {
-      auto status = it->second.data();
+      std::string status = it->second.data();
 
-      if (status == std::string("OK")) {
+      // TODO monero-project empty string status is returned for download update response when an update is available
+      if (status == std::string("OK") || status == std::string("")) {
         return;
       }
       else throw PyMoneroRpcError(status);

src/cpp/daemon/py_monero_daemon_rpc.h

@@ -2,23 +2,17 @@
 
 #include "py_monero_daemon.h"
 
-class PyMoneroDaemonPoller {
+class PyMoneroDaemonPoller: public PyThreadPoller {
 public:
 
-  ~PyMoneroDaemonPoller();
   PyMoneroDaemonPoller(PyMoneroDaemon* daemon, uint64_t poll_period_ms = 5000);
 
-  void set_is_polling(bool is_polling);
+  void poll() override;
 
 private:
   PyMoneroDaemon* m_daemon;
   std::shared_ptr<monero::monero_block_header> m_last_header;
-  uint64_t m_poll_period_ms;
-  std::atomic<bool> m_is_polling;
-  std::thread m_thread;
 
-  void loop();
-  void poll();
   void announce_block_header(const std::shared_ptr<monero::monero_block_header>& header);
 };
 
@@ -29,7 +23,7 @@ class PyMoneroDaemonRpc : public PyMoneroDaemon {
   PyMoneroDaemonRpc(const std::shared_ptr<PyMoneroRpcConnection>& rpc);
   PyMoneroDaemonRpc(const std::string& uri, const std::string& username = "", const std::string& password = "", const std::string& proxy_uri = "", const std::string& zmq_uri = "", uint64_t timeout = 20000);
 
-  std::vector<std::shared_ptr<PyMoneroDaemonListener>> get_listeners() override { return m_listeners; }
+  std::vector<std::shared_ptr<PyMoneroDaemonListener>> get_listeners() override;
   void add_listener(const std::shared_ptr<PyMoneroDaemonListener> &listener) override;
   void remove_listener(const std::shared_ptr<PyMoneroDaemonListener> &listener) override;
   void remove_listeners() override;
@@ -90,8 +84,7 @@ class PyMoneroDaemonRpc : public PyMoneroDaemon {
   void submit_blocks(const std::vector<std::string>& block_blobs) override;
   std::shared_ptr<PyMoneroPruneResult> prune_blockchain(bool check) override;
   std::shared_ptr<PyMoneroDaemonUpdateCheckResult> check_for_update() override;
-  std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> download_update(const std::string& path) override;
-  std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> download_update() override;
+  std::shared_ptr<PyMoneroDaemonUpdateDownloadResult> download_update(const std::string& path = "") override;
   void stop() override;
   std::shared_ptr<monero::monero_block_header> wait_for_next_block_header();
   static void check_response_status(const std::shared_ptr<PyMoneroPathResponse>& response);

src/cpp/py_monero.cpp

@@ -1546,12 +1546,9 @@ PYBIND11_MODULE(monero, m) {
     .def("check_for_update", [](PyMoneroDaemon& self) {
       MONERO_CATCH_AND_RETHROW(self.check_for_update());
     })
-    .def("download_update", [](PyMoneroDaemon& self) {
-      MONERO_CATCH_AND_RETHROW(self.download_update());
-    })
-    .def("download_update", [](PyMoneroDaemon& self, const std::string& download_path) {
-      MONERO_CATCH_AND_RETHROW(self.download_update(download_path));
-    }, py::arg("download_path"))
+    .def("download_update", [](PyMoneroDaemon& self, const std::string& path) {
+      MONERO_CATCH_AND_RETHROW(self.download_update(path));
+    }, py::arg("path") = "")
     .def("stop", [](PyMoneroDaemon& self) {
       MONERO_CATCH_AND_RETHROW(self.stop());
     })

src/cpp/wallet/py_monero_wallet_rpc.cpp

@@ -3,30 +3,8 @@
 
 PyMoneroWalletPoller::PyMoneroWalletPoller(PyMoneroWallet *wallet) {
   m_wallet = wallet;
-  m_is_polling = false;
   m_num_polling = 0;
-}
-
-PyMoneroWalletPoller::~PyMoneroWalletPoller() {
-  set_is_polling(false);
-}
-
-void PyMoneroWalletPoller::set_is_polling(bool is_polling) {
-  if (is_polling == m_is_polling) return;
-  m_is_polling = is_polling;
-
-  if (m_is_polling) {
-    m_thread = std::thread([this]() {
-      loop();
-    });
-    m_thread.detach();
-  } else {
-    if (m_thread.joinable()) m_thread.join();
-  }
-}
-
-void PyMoneroWalletPoller::set_period_in_ms(uint64_t period_ms) {
-  m_poll_period_ms = period_ms;
+  init_common("monero_wallet_rpc");
 }
 
 void PyMoneroWalletPoller::poll() {
@@ -146,18 +124,6 @@ std::shared_ptr<monero::monero_tx_wallet> PyMoneroWalletPoller::get_tx(const std
   return nullptr;
 }
 
-void PyMoneroWalletPoller::loop() {
-  while (m_is_polling) {
-    try {
-      poll();
-    } catch (const std::exception& e) {
-      std::cout << "ERROR " << e.what() << std::endl;
-    }
-
-    std::this_thread::sleep_for(std::chrono::milliseconds(m_poll_period_ms));
-  }
-}
-
 void PyMoneroWalletPoller::on_new_block(uint64_t height) {
   m_wallet->announce_new_block(height);
 }
@@ -1733,6 +1699,7 @@ bool PyMoneroWalletRpc::is_closed() const {
 }
 
 void PyMoneroWalletRpc::close(bool save) {
+  MTRACE("PyMoneroWalletRpc::close()");
   clear();
   auto params = std::make_shared<PyMoneroCloseWalletParams>(save);
   PyMoneroJsonRequest request("close_wallet", params);
@@ -1945,7 +1912,9 @@ void PyMoneroWalletRpc::refresh_listening() {
 }
 
 void PyMoneroWalletRpc::poll() {
-  if (m_poller != nullptr && m_poller->is_polling()) m_poller->poll();
+  if (m_poller != nullptr && m_poller->is_polling()) {
+    m_poller->poll();
+  }
 }
 
 void PyMoneroWalletRpc::clear() {

src/cpp/wallet/py_monero_wallet_rpc.h

@@ -3,33 +3,23 @@
 #include "py_monero_wallet.h"
 
 
-class PyMoneroWalletPoller {
+class PyMoneroWalletPoller: public PyThreadPoller {
 public:
 
-  ~PyMoneroWalletPoller();
   PyMoneroWalletPoller(PyMoneroWallet *wallet);
+  void poll() override;
 
-  bool is_polling() const { return m_is_polling; }
-  void set_is_polling(bool is_polling);
-  void set_period_in_ms(uint64_t period_ms);
-  void poll();
-
-protected:
-  mutable boost::recursive_mutex m_mutex;
+private:
   PyMoneroWallet *m_wallet;
-  std::atomic<bool> m_is_polling;
-  uint64_t m_poll_period_ms = 20000;
-  std::thread m_thread;
-  int m_num_polling;
+  std::atomic<int> m_num_polling;
+
   std::vector<std::string> m_prev_unconfirmed_notifications;
   std::vector<std::string> m_prev_confirmed_notifications;
-
   boost::optional<std::shared_ptr<PyMoneroWalletBalance>> m_prev_balances;
   boost::optional<uint64_t> m_prev_height;
   std::vector<std::shared_ptr<monero::monero_tx_wallet>> m_prev_locked_txs;
 
   std::shared_ptr<monero::monero_tx_wallet> get_tx(const std::vector<std::shared_ptr<monero::monero_tx_wallet>>& txs, const std::string& tx_hash);
-  void loop();
   void on_new_block(uint64_t height);
   void notify_outputs(const std::shared_ptr<monero::monero_tx_wallet> &tx);
   bool check_for_changed_balances();

src/python/monero_daemon.pyi

@@ -48,20 +48,11 @@ class MoneroDaemon:
         :return MoneroDaemonUpdateCheckResult: the result of the update check
         """
         ...
-    @typing.overload
-    def download_update(self) -> MoneroDaemonUpdateDownloadResult:
-        """
-        Download an update.
-
-        :param path: is the path to download the update (optional)
-        :return MoneroDaemonUpdateDownloadResult: the result of the update download
-        """
-        ...
-    @typing.overload
-    def download_update(self, download_path: str) -> MoneroDaemonUpdateDownloadResult:
+    def download_update(self, path: str = '') -> MoneroDaemonUpdateDownloadResult:
         """
         Download an update.
-
+
+        :param str path: download path.
         :return MoneroDaemonUpdateDownloadResult: the result of the update download
         """
         ...