diff --git a/src/slic3r/GUI/UserAccountCommunication.cpp b/src/slic3r/GUI/UserAccountCommunication.cpp index 0268da1d74..ba736891c1 100644 --- a/src/slic3r/GUI/UserAccountCommunication.cpp +++ b/src/slic3r/GUI/UserAccountCommunication.cpp @@ -242,8 +242,6 @@ void UserAccountCommunication::set_username(const std::string& username) { m_username = username; { - // We don't need mutex lock here, as credentials are guarded by own mutex in m_session - //std::lock_guard lock(m_session_mutex); if (is_secret_store_ok()) { std::string tokens; if (m_remember_session) { @@ -292,36 +290,28 @@ void UserAccountCommunication::set_remember_session(bool b) std::string UserAccountCommunication::get_access_token() { - { - // We don't need mutex lock here, as credentials are guarded by own mutex in m_session - //std::lock_guard lock(m_session_mutex); - return m_session->get_access_token(); - } + + return m_session->get_access_token(); + } std::string UserAccountCommunication::get_shared_session_key() { - { - // We don't need mutex lock here, as credentials are guarded by own mutex in m_session - //std::lock_guard lock(m_session_mutex); - return m_session->get_shared_session_key(); - } + + return m_session->get_shared_session_key(); + } void UserAccountCommunication::set_polling_enabled(bool enabled) { - { - std::lock_guard lock(m_session_mutex); - return m_session->set_polling_action(enabled ? UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_PRINTER_MODELS : UserAccountActionID::USER_ACCOUNT_ACTION_DUMMY); - } + return m_session->set_polling_action(enabled ? UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_PRINTER_MODELS : UserAccountActionID::USER_ACCOUNT_ACTION_DUMMY); } void UserAccountCommunication::on_uuid_map_success() { - { - std::lock_guard lock(m_session_mutex); - return m_session->set_polling_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_STATUS); - } + + return m_session->set_polling_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_STATUS); + } // Generates and stores Code Verifier - second call deletes previous one. @@ -371,13 +361,10 @@ bool UserAccountCommunication::is_logged() } void UserAccountCommunication::do_login() { - { - std::lock_guard lock(m_session_mutex); - if (!m_session->is_initialized()) { - login_redirect(); - } else { - m_session->enqueue_test_with_refresh(); - } + if (!m_session->is_initialized()) { + login_redirect(); + } else { + m_session->enqueue_test_with_refresh(); } wakeup_session_thread(); } @@ -389,85 +376,64 @@ void UserAccountCommunication::do_logout() void UserAccountCommunication::do_clear() { - { - std::lock_guard lock(m_session_mutex); - m_session->clear(); - } + m_session->clear(); set_username({}); m_token_timer->Stop(); } void UserAccountCommunication::on_login_code_recieved(const std::string& url_message) { - { - std::lock_guard lock(m_session_mutex); - const std::string code = get_code_from_message(url_message); - m_session->init_with_code(code, m_code_verifier); - } + const std::string code = get_code_from_message(url_message); + m_session->init_with_code(code, m_code_verifier); wakeup_session_thread(); } void UserAccountCommunication::enqueue_connect_printer_models_action() { - { - std::lock_guard lock(m_session_mutex); - if (!m_session->is_initialized()) { - BOOST_LOG_TRIVIAL(error) << "Connect Printer Models connection failed - Not Logged in."; - return; - } - m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_PRINTER_MODELS, nullptr, nullptr, {}); + if (!m_session->is_initialized()) { + BOOST_LOG_TRIVIAL(error) << "Connect Printer Models connection failed - Not Logged in."; + return; } + m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_PRINTER_MODELS, nullptr, nullptr, {}); wakeup_session_thread(); } void UserAccountCommunication::enqueue_connect_status_action() { - { - std::lock_guard lock(m_session_mutex); - if (!m_session->is_initialized()) { - BOOST_LOG_TRIVIAL(error) << "Connect Status endpoint connection failed - Not Logged in."; - return; - } - m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_STATUS, nullptr, nullptr, {}); + if (!m_session->is_initialized()) { + BOOST_LOG_TRIVIAL(error) << "Connect Status endpoint connection failed - Not Logged in."; + return; } + m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_STATUS, nullptr, nullptr, {}); wakeup_session_thread(); } void UserAccountCommunication::enqueue_test_connection() { - { - std::lock_guard lock(m_session_mutex); - if (!m_session->is_initialized()) { - BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; - return; - } - m_session->enqueue_test_with_refresh(); + if (!m_session->is_initialized()) { + BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; + return; } + m_session->enqueue_test_with_refresh(); wakeup_session_thread(); } void UserAccountCommunication::enqueue_avatar_action(const std::string& url) { - { - std::lock_guard lock(m_session_mutex); - if (!m_session->is_initialized()) { - BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; - return; - } - m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_AVATAR, nullptr, nullptr, url); + if (!m_session->is_initialized()) { + BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; + return; } + m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_AVATAR, nullptr, nullptr, url); wakeup_session_thread(); } void UserAccountCommunication::enqueue_printer_data_action(const std::string& uuid) { - { - std::lock_guard lock(m_session_mutex); - if (!m_session->is_initialized()) { - BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; - return; - } - m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_DATA_FROM_UUID, nullptr, nullptr, uuid); + if (!m_session->is_initialized()) { + BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; + return; } + m_session->enqueue_action(UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_DATA_FROM_UUID, nullptr, nullptr, uuid); wakeup_session_thread(); } @@ -479,18 +445,15 @@ void UserAccountCommunication::request_refresh() void UserAccountCommunication::enqueue_refresh() { - { - std::lock_guard lock(m_session_mutex); - if (!m_session->is_initialized()) { - BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; - return; - } - if (m_session->is_enqueued(UserAccountActionID::USER_ACCOUNT_ACTION_REFRESH_TOKEN)) { - BOOST_LOG_TRIVIAL(debug) << "User Account: Token refresh already enqueued, skipping..."; - return; - } - m_session->enqueue_refresh({}); + if (!m_session->is_initialized()) { + BOOST_LOG_TRIVIAL(error) << "Connect Printers endpoint connection failed - Not Logged in."; + return; } + if (m_session->is_enqueued(UserAccountActionID::USER_ACCOUNT_ACTION_REFRESH_TOKEN)) { + BOOST_LOG_TRIVIAL(debug) << "User Account: Token refresh already enqueued, skipping..."; + return; + } + m_session->enqueue_refresh({}); wakeup_session_thread(); } @@ -512,10 +475,7 @@ void UserAccountCommunication::init_session_thread() continue; } m_thread_wakeup = false; - { - std::lock_guard lock(m_session_mutex); - m_session->process_action_queue(); - } + m_session->process_action_queue(); } }); } diff --git a/src/slic3r/GUI/UserAccountCommunication.hpp b/src/slic3r/GUI/UserAccountCommunication.hpp index fdd4ff147a..96904e28a5 100644 --- a/src/slic3r/GUI/UserAccountCommunication.hpp +++ b/src/slic3r/GUI/UserAccountCommunication.hpp @@ -81,7 +81,6 @@ public: private: std::unique_ptr m_session; std::thread m_thread; - std::mutex m_session_mutex; std::mutex m_thread_stop_mutex; std::condition_variable m_thread_stop_condition; bool m_thread_stop { false }; diff --git a/src/slic3r/GUI/UserAccountSession.cpp b/src/slic3r/GUI/UserAccountSession.cpp index 55e66f6d5b..5d94542258 100644 --- a/src/slic3r/GUI/UserAccountSession.cpp +++ b/src/slic3r/GUI/UserAccountSession.cpp @@ -35,6 +35,7 @@ wxDEFINE_EVENT(EVT_UA_ENQUEUED_REFRESH, SimpleEvent); void UserActionPost::perform(/*UNUSED*/ wxEvtHandler* evt_handler, /*UNUSED*/ const std::string& access_token, UserActionSuccessFn success_callback, UserActionFailFn fail_callback, const std::string& input) const { std::string url = m_url; + BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << " " << url; auto http = Http::post(std::move(url)); if (!input.empty()) http.set_post_body(input); @@ -53,6 +54,7 @@ void UserActionPost::perform(/*UNUSED*/ wxEvtHandler* evt_handler, /*UNUSED*/ co void UserActionGetWithEvent::perform(wxEvtHandler* evt_handler, const std::string& access_token, UserActionSuccessFn success_callback, UserActionFailFn fail_callback, const std::string& input) const { std::string url = m_url + input; + BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << " " << url; auto http = Http::get(std::move(url)); if (!access_token.empty()) { http.header("Authorization", "Bearer " + access_token); @@ -82,75 +84,98 @@ void UserActionGetWithEvent::perform(wxEvtHandler* evt_handler, const std::strin http.perform_sync(HttpRetryOpt::default_retry()); } -bool UserAccountSession::is_enqueued(UserAccountActionID action_id) const { - return std::any_of( - std::begin(m_priority_action_queue), std::end(m_priority_action_queue), - [action_id](const ActionQueueData& item) { return item.action_id == action_id; } - ); +bool UserAccountSession::is_enqueued(UserAccountActionID action_id) const +{ + { + std::lock_guard lock(m_session_mutex); + return std::any_of( + std::begin(m_priority_action_queue), std::end(m_priority_action_queue), + [action_id](const ActionQueueData& item) { return item.action_id == action_id; } + ); + } } - void UserAccountSession::process_action_queue() { - if (!m_proccessing_enabled) - return; - if (m_priority_action_queue.empty() && m_action_queue.empty()) { - // update printers periodically - enqueue_action(m_polling_action, nullptr, nullptr, {}); - } - // priority queue works even when tokens are empty or broken - while (!m_priority_action_queue.empty()) { - std::string access_token; - { - std::lock_guard lock(m_credentials_mutex); - access_token = m_access_token; + { + std::lock_guard lock(m_session_mutex); + if (!m_proccessing_enabled) + return; + if (m_priority_action_queue.empty() && m_action_queue.empty()) { + // update printers periodically + enqueue_action_inner(m_polling_action, nullptr, nullptr, {}); } - m_actions[m_priority_action_queue.front().action_id]->perform(p_evt_handler, access_token, m_priority_action_queue.front().success_callback, m_priority_action_queue.front().fail_callback, m_priority_action_queue.front().input); - if (!m_priority_action_queue.empty()) + } + process_action_queue_inner(); +} +void UserAccountSession::process_action_queue_inner() +{ + bool call_priority = false; + bool call_standard = false; + ActionQueueData selected_data; + { + std::lock_guard lock(m_session_mutex); + + // priority queue works even when tokens are empty or broken + if (!m_priority_action_queue.empty()) { + // Do a copy here even its costly. We need to get data outside m_session_mutex protected code to perform background operation over it. + selected_data = m_priority_action_queue.front(); m_priority_action_queue.pop_front(); - } - // regular queue has to wait until priority fills tokens - if (!this->is_initialized()) - return; - while (!m_action_queue.empty()) { - std::string access_token; - { - std::lock_guard lock(m_credentials_mutex); - access_token = m_access_token; - } - m_actions[m_action_queue.front().action_id]->perform(p_evt_handler, access_token, m_action_queue.front().success_callback, m_action_queue.front().fail_callback, m_action_queue.front().input); - if (!m_action_queue.empty()) + call_priority = true; + } else if (this->is_initialized() && !m_action_queue.empty()) { + // regular queue has to wait until priority fills tokens + // Do a copy here even its costly. We need to get data outside m_session_mutex protected code to perform background operation over it. + selected_data = m_action_queue.front(); m_action_queue.pop(); + call_standard = true; + } } + if (call_priority || call_standard) { + m_actions[selected_data.action_id]->perform(p_evt_handler, get_access_token(), selected_data.success_callback, selected_data.fail_callback, selected_data.input); + process_action_queue_inner(); + } } void UserAccountSession::enqueue_action(UserAccountActionID id, UserActionSuccessFn success_callback, UserActionFailFn fail_callback, const std::string& input) { - m_proccessing_enabled = true; - m_action_queue.push({ id, success_callback, fail_callback, input }); + { + std::lock_guard lock(m_session_mutex); + enqueue_action_inner(id, success_callback, fail_callback, input); + } } +// called from m_session_mutex protected code only! +void UserAccountSession::enqueue_action_inner(UserAccountActionID id, UserActionSuccessFn success_callback, UserActionFailFn fail_callback, const std::string& input) +{ + m_proccessing_enabled = true; + m_action_queue.push({ id, success_callback, fail_callback, input }); +} void UserAccountSession::init_with_code(const std::string& code, const std::string& code_verifier) { - // Data we have - const std::string REDIRECT_URI = "prusaslicer://login"; - std::string post_fields = "code=" + code + - "&client_id=" + client_id() + - "&grant_type=authorization_code" + - "&redirect_uri=" + REDIRECT_URI + - "&code_verifier="+ code_verifier; + { + std::lock_guard lock(m_session_mutex); + // Data we have + const std::string REDIRECT_URI = "prusaslicer://login"; + std::string post_fields = "code=" + code + + "&client_id=" + client_id() + + "&grant_type=authorization_code" + + "&redirect_uri=" + REDIRECT_URI + + "&code_verifier="+ code_verifier; - m_proccessing_enabled = true; - // fail fn might be cancel_queue here - m_priority_action_queue.push_back({ UserAccountActionID::USER_ACCOUNT_ACTION_CODE_FOR_TOKEN - , std::bind(&UserAccountSession::token_success_callback, this, std::placeholders::_1) - , std::bind(&UserAccountSession::code_exchange_fail_callback, this, std::placeholders::_1) - , post_fields }); + m_proccessing_enabled = true; + // fail fn might be cancel_queue here + m_priority_action_queue.push_back({ UserAccountActionID::USER_ACCOUNT_ACTION_CODE_FOR_TOKEN + , std::bind(&UserAccountSession::token_success_callback, this, std::placeholders::_1) + , std::bind(&UserAccountSession::code_exchange_fail_callback, this, std::placeholders::_1) + , post_fields }); + } } void UserAccountSession::token_success_callback(const std::string& body) { + // No need to use lock m_session_mutex here + BOOST_LOG_TRIVIAL(debug) << "Access token refreshed"; // Data we need std::string access_token, refresh_token, shared_session_key; @@ -204,21 +229,24 @@ void UserAccountSession::token_success_callback(const std::string& body) void UserAccountSession::code_exchange_fail_callback(const std::string& body) { + BOOST_LOG_TRIVIAL(debug) << "Access token refresh failed, body: " << body; clear(); cancel_queue(); // Unlike refresh_fail_callback, no event was triggered so far, do it. (USER_ACCOUNT_ACTION_CODE_FOR_TOKEN does not send events) - wxQueueEvent(p_evt_handler, new UserAccountFailEvent(EVT_UA_RESET, std::move(body))); + wxQueueEvent(p_evt_handler, new UserAccountFailEvent(EVT_UA_RESET, body)); } void UserAccountSession::enqueue_test_with_refresh() { - // on test fail - try refresh - m_proccessing_enabled = true; - m_priority_action_queue.push_back({ UserAccountActionID::USER_ACCOUNT_ACTION_TEST_ACCESS_TOKEN, nullptr, std::bind(&UserAccountSession::enqueue_refresh, this, std::placeholders::_1), {} }); + { + std::lock_guard lock(m_session_mutex); + // on test fail - try refresh + m_proccessing_enabled = true; + m_priority_action_queue.push_back({ UserAccountActionID::USER_ACCOUNT_ACTION_TEST_ACCESS_TOKEN, nullptr, std::bind(&UserAccountSession::enqueue_refresh, this, std::placeholders::_1), {} }); + } } - void UserAccountSession::enqueue_refresh(const std::string& body) { wxQueueEvent(p_evt_handler, new SimpleEvent(EVT_UA_ENQUEUED_REFRESH)); @@ -230,11 +258,13 @@ void UserAccountSession::enqueue_refresh(const std::string& body) "&client_id=" + client_id() + "&refresh_token=" + m_refresh_token; } - - m_priority_action_queue.push_back({ UserAccountActionID::USER_ACCOUNT_ACTION_REFRESH_TOKEN - , std::bind(&UserAccountSession::token_success_callback, this, std::placeholders::_1) - , std::bind(&UserAccountSession::refresh_fail_callback, this, std::placeholders::_1) - , post_fields }); + { + std::lock_guard lock(m_session_mutex); + m_priority_action_queue.push_back({ UserAccountActionID::USER_ACCOUNT_ACTION_REFRESH_TOKEN + , std::bind(&UserAccountSession::token_success_callback, this, std::placeholders::_1) + , std::bind(&UserAccountSession::refresh_fail_callback, this, std::placeholders::_1) + , post_fields }); + } } void UserAccountSession::refresh_fail_callback(const std::string& body) @@ -244,15 +274,17 @@ void UserAccountSession::refresh_fail_callback(const std::string& body) // No need to notify UI thread here // backtrace: load tokens -> TEST_TOKEN fail (access token bad) -> REFRESH_TOKEN fail (refresh token bad) // USER_ACCOUNT_ACTION_TEST_ACCESS_TOKEN triggers EVT_UA_FAIL, we need also RESET - wxQueueEvent(p_evt_handler, new UserAccountFailEvent(EVT_UA_RESET, std::move(body))); - + wxQueueEvent(p_evt_handler, new UserAccountFailEvent(EVT_UA_RESET, body)); } void UserAccountSession::cancel_queue() { - m_priority_action_queue.clear(); - while (!m_action_queue.empty()) { - m_action_queue.pop(); + { + std::lock_guard lock(m_session_mutex); + m_priority_action_queue.clear(); + while (!m_action_queue.empty()) { + m_action_queue.pop(); + } } } diff --git a/src/slic3r/GUI/UserAccountSession.hpp b/src/slic3r/GUI/UserAccountSession.hpp index ec1924778c..8b807aeff3 100644 --- a/src/slic3r/GUI/UserAccountSession.hpp +++ b/src/slic3r/GUI/UserAccountSession.hpp @@ -143,7 +143,10 @@ public: m_refresh_token.clear(); m_shared_session_key.clear(); } - m_proccessing_enabled = false; + { + std::lock_guard lock(m_session_mutex); + m_proccessing_enabled = false; + } } // Functions that automatically enable action queu processing @@ -152,8 +155,8 @@ public: // Special enques, that sets callbacks. void enqueue_test_with_refresh(); void enqueue_refresh(const std::string& body); - void process_action_queue(); + bool is_initialized() const { std::lock_guard lock(m_credentials_mutex); return !m_access_token.empty() || !m_refresh_token.empty(); @@ -177,21 +180,22 @@ public: } //void set_polling_enabled(bool enabled) {m_polling_action = enabled ? UserAccountActionID::USER_ACCOUNT_ACTION_CONNECT_PRINTER_MODELS : UserAccountActionID::USER_ACCOUNT_ACTION_DUMMY; } - void set_polling_action(UserAccountActionID action) { m_polling_action = action; } + void set_polling_action(UserAccountActionID action) { + std::lock_guard lock(m_session_mutex); + m_polling_action = action; + } private: - void refresh_fail_callback(const std::string& body); void cancel_queue(); void code_exchange_fail_callback(const std::string& body); void token_success_callback(const std::string& body); std::string client_id() const { return Utils::ServiceConfig::instance().account_client_id(); } + void process_action_queue_inner(); - // false prevents action queu to be processed - no communication is done - // sets to true by init_with_code or enqueue_action call - bool m_proccessing_enabled {false}; - // action when woken up on idle - switches between USER_ACCOUNT_ACTION_CONNECT_PRINTER_MODELS and USER_ACCOUNT_ACTION_CONNECT_STATUS - // set to USER_ACCOUNT_ACTION_DUMMY to switch off polling - UserAccountActionID m_polling_action; + // called from m_session_mutex protected code only + void enqueue_action_inner(UserAccountActionID id, UserActionSuccessFn success_callback, UserActionFailFn fail_callback, const std::string& input); + + // Section of following vars is guarded by this mutex mutable std::mutex m_credentials_mutex; @@ -201,8 +205,21 @@ private: long long m_next_token_timeout; // End of section guarded by m_credentials_mutex + + // Section of following vars is guarded by this mutex + mutable std::mutex m_session_mutex; + std::queue m_action_queue; - std::deque m_priority_action_queue; + std::deque m_priority_action_queue; + // false prevents action queue to be processed - no communication is done + // sets to true by init_with_code or enqueue_action call + bool m_proccessing_enabled {false}; + // action when woken up on idle - switches between USER_ACCOUNT_ACTION_CONNECT_PRINTER_MODELS and USER_ACCOUNT_ACTION_CONNECT_STATUS + // set to USER_ACCOUNT_ACTION_DUMMY to switch off polling + UserAccountActionID m_polling_action; + + // End of section guarded by m_session_mutex + std::map> m_actions; wxEvtHandler* p_evt_handler;