From 431943408b2a6a4b4bde14275f5554e861c7d4ea Mon Sep 17 00:00:00 2001 From: SoftFever Date: Mon, 2 Feb 2026 18:12:19 +0800 Subject: [PATCH] fix UI block issues --- src/slic3r/GUI/DeviceCore/DevManager.cpp | 5 +- src/slic3r/GUI/GUI_App.cpp | 11 ++- src/slic3r/Utils/MoonrakerPrinterAgent.cpp | 101 +++++++++++---------- src/slic3r/Utils/MoonrakerPrinterAgent.hpp | 13 +-- src/slic3r/Utils/NetworkAgent.cpp | 8 ++ src/slic3r/Utils/NetworkAgentFactory.cpp | 41 ++++++++- src/slic3r/Utils/NetworkAgentFactory.hpp | 10 ++ 7 files changed, 124 insertions(+), 65 deletions(-) diff --git a/src/slic3r/GUI/DeviceCore/DevManager.cpp b/src/slic3r/GUI/DeviceCore/DevManager.cpp index 6f4885f4cc..df161d9dea 100644 --- a/src/slic3r/GUI/DeviceCore/DevManager.cpp +++ b/src/slic3r/GUI/DeviceCore/DevManager.cpp @@ -858,7 +858,10 @@ namespace Slic3r { if (MachineObject* obj_ = get_selected_machine()) { GUI::wxGetApp().sidebar().update_sync_status(obj_); - GUI::wxGetApp().sidebar().load_ams_list(obj_); + if(m_agent->get_filament_sync_mode() == FilamentSyncMode::subscription) + { + GUI::wxGetApp().sidebar().load_ams_list(obj_); + } }; } diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index bbd4c26974..a7a0d49fa0 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -2591,6 +2591,11 @@ int GUI_App::OnExit() m_user_manager = nullptr; } + // Clear the printer agent cache before destroying the NetworkAgent. + // This disconnects all cached agents and releases their shared_ptrs, + // ensuring clean thread shutdown before the agent is deleted. + NetworkAgentFactory::clear_printer_agent_cache(); + if (m_agent) { // BBS avoid a crash on mac platform #ifdef __WINDOWS__ @@ -3515,10 +3520,10 @@ void GUI_App::switch_printer_agent() sidebar().update_all_preset_comboboxes(); BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ": printer agent switched to " << effective_agent_id; - } - // Auto-switch MachineObject (runs even if agent IDs match) - select_machine(effective_agent_id); + // Auto-switch MachineObject + select_machine(effective_agent_id); + } } void GUI_App::select_machine(const std::string& agent_id) diff --git a/src/slic3r/Utils/MoonrakerPrinterAgent.cpp b/src/slic3r/Utils/MoonrakerPrinterAgent.cpp index 6a86224bf9..ee1b2b6235 100644 --- a/src/slic3r/Utils/MoonrakerPrinterAgent.cpp +++ b/src/slic3r/Utils/MoonrakerPrinterAgent.cpp @@ -98,7 +98,15 @@ MoonrakerPrinterAgent::MoonrakerPrinterAgent(std::string log_dir) : m_cloud_agen MoonrakerPrinterAgent::~MoonrakerPrinterAgent() { - disconnect_printer(); // This will handle thread cleanup + { + std::lock_guard lock(connect_mutex); + device_info = MoonrakerDeviceInfo{}; + ++connect_generation; + } + if (connect_thread.joinable()) { + connect_thread.join(); + } + stop_status_stream(); } AgentInfo MoonrakerPrinterAgent::get_agent_info_static() @@ -133,31 +141,20 @@ int MoonrakerPrinterAgent::connect_printer(std::string dev_id, std::string dev_i return BAMBU_NETWORK_ERR_INVALID_HANDLE; } - // Check if connection already in progress + std::string base_url; + std::string api_key; + uint64_t gen; { std::lock_guard lock(connect_mutex); init_device_info(dev_id, dev_ip, username, password, use_ssl); - if (connect_in_progress.load()) { - // Don't reject - wait for previous connection to complete - // This can happen if MonitorPanel triggers connect while previous connect is still running - } else { - connect_in_progress.store(true); - connect_stop_requested.store(false); + gen = ++connect_generation; + base_url = device_info.base_url; + api_key = device_info.api_key; + if (connect_thread.joinable()) { + connect_thread.detach(); } } - // Wait for previous connection thread to finish - if (connect_thread.joinable()) { - connect_thread.join(); - } - - // Now we can start a new connection - { - std::lock_guard lock(connect_mutex); - connect_in_progress.store(true); - connect_stop_requested.store(false); - } - // Stop existing status stream and clear state stop_status_stream(); { @@ -168,30 +165,26 @@ int MoonrakerPrinterAgent::connect_printer(std::string dev_id, std::string dev_i ws_last_dispatch_ms.store(0); last_print_state.clear(); - // Launch connection in background thread - connect_thread = std::thread([this, dev_id]() { perform_connection_async(dev_id, device_info.base_url, device_info.api_key); }); + // Launch connection in background thread (capture by value to avoid data races) + { + std::lock_guard lock(connect_mutex); + connect_thread = std::thread([this, dev_id, base_url, api_key, gen]() { perform_connection_async(dev_id, base_url, api_key, gen); }); + } return BAMBU_NETWORK_SUCCESS; } int MoonrakerPrinterAgent::disconnect_printer() { - // Stop connection thread if running { std::lock_guard lock(connect_mutex); device_info = MoonrakerDeviceInfo{}; - if (connect_in_progress.load()) { - connect_stop_requested.store(true); - // Wake up any sleeping - connect_cv.notify_all(); + ++connect_generation; // Invalidate any in-flight connection + if (connect_thread.joinable()) { + connect_thread.detach(); } } - // Wait for connection thread to finish (with timeout) - if (connect_thread.joinable()) { - connect_thread.join(); - } - stop_status_stream(); return BAMBU_NETWORK_SUCCESS; } @@ -1842,14 +1835,12 @@ int MoonrakerPrinterAgent::pause_print(const std::string& dev_id) int MoonrakerPrinterAgent::resume_print(const std::string& dev_id) { - std::string gcode = "RESUME"; - return send_gcode(dev_id, gcode) ? BAMBU_NETWORK_SUCCESS : BAMBU_NETWORK_ERR_SEND_MSG_FAILED; + return send_gcode(dev_id, "RESUME") ? BAMBU_NETWORK_SUCCESS : BAMBU_NETWORK_ERR_SEND_MSG_FAILED; } int MoonrakerPrinterAgent::cancel_print(const std::string& dev_id) { - std::string gcode = "CANCEL_PRINT"; - return send_gcode(dev_id, gcode) ? BAMBU_NETWORK_SUCCESS : BAMBU_NETWORK_ERR_SEND_MSG_FAILED; + return send_gcode(dev_id, "CANCEL_PRINT") ? BAMBU_NETWORK_SUCCESS : BAMBU_NETWORK_ERR_SEND_MSG_FAILED; } bool MoonrakerPrinterAgent::send_jsonrpc_command(const std::string& base_url, @@ -1889,20 +1880,38 @@ bool MoonrakerPrinterAgent::send_jsonrpc_command(const std::string& base_url, return success; } -void MoonrakerPrinterAgent::perform_connection_async(const std::string& dev_id, const std::string& base_url, const std::string& api_key) +void MoonrakerPrinterAgent::perform_connection_async(const std::string& dev_id, const std::string& base_url, const std::string& api_key, uint64_t generation) { + auto is_stale = [&]() { return generation != connect_generation.load(); }; + int result = BAMBU_NETWORK_ERR_CONNECTION_TO_PRINTER_FAILED; std::string error_msg; + // Early exit if a newer connection was started before we begin + if (is_stale()) { + return; + } + try { - if (!fetch_device_info(base_url, api_key, device_info, error_msg)) { + MoonrakerDeviceInfo fetched_info; + if (!fetch_device_info(base_url, api_key, fetched_info, error_msg)) { BOOST_LOG_TRIVIAL(error) << "MoonrakerPrinterAgent: Failed to fetch server info: " << error_msg; // Orca todo: revist here, for now don't send error, this is set current MachineObject to null // dispatch_local_connect(ConnectStatusFailed, dev_id, "server_info_failed"); - finish_connection(); return; } + // Commit fetched info back to device_info under lock, only if still current + { + std::lock_guard lock(connect_mutex); + if (is_stale()) { + return; + } + device_info.dev_name = fetched_info.dev_name; + device_info.version = fetched_info.version; + device_info.klippy_state = fetched_info.klippy_state; + } + // Orca todo: disable websocket for now, as we don't use MonitorPanel for Moonraker printers yet #if 0 // Query initial status @@ -1929,23 +1938,15 @@ void MoonrakerPrinterAgent::perform_connection_async(const std::string& dev_id, result = BAMBU_NETWORK_ERR_CONNECTION_TO_PRINTER_FAILED; } - // Dispatch final result to UI - if (result == BAMBU_NETWORK_SUCCESS) { + // Only dispatch if this connection is still the current one + if (result == BAMBU_NETWORK_SUCCESS && !is_stale()) { dispatch_local_connect(ConnectStatusOk, dev_id, "0"); dispatch_printer_connected(dev_id); BOOST_LOG_TRIVIAL(info) << "MoonrakerPrinterAgent: connect_printer completed - dev_id=" << dev_id; - } else if (result != BAMBU_NETWORK_ERR_CANCELED) { + } else if (result != BAMBU_NETWORK_SUCCESS && result != BAMBU_NETWORK_ERR_CANCELED) { // Orca todo: revist here, for now don't send error, this is set current MachineObject to null // dispatch_local_connect(ConnectStatusFailed, dev_id, error_msg); } - - finish_connection(); -} - -void MoonrakerPrinterAgent::finish_connection() -{ - std::lock_guard lock(connect_mutex); - connect_in_progress.store(false); } bool MoonrakerPrinterAgent::is_numeric(const std::string& value) diff --git a/src/slic3r/Utils/MoonrakerPrinterAgent.hpp b/src/slic3r/Utils/MoonrakerPrinterAgent.hpp index 0a560388cf..bddf25296a 100644 --- a/src/slic3r/Utils/MoonrakerPrinterAgent.hpp +++ b/src/slic3r/Utils/MoonrakerPrinterAgent.hpp @@ -4,7 +4,6 @@ #include "IPrinterAgent.hpp" #include "ICloudServiceAgent.hpp" -#include #include #include #include @@ -158,8 +157,8 @@ private: // Connection thread management void perform_connection_async(const std::string& dev_id, const std::string& base_url, - const std::string& api_key); - void finish_connection(); + const std::string& api_key, + uint64_t generation); std::string ssdp_announced_host; std::string ssdp_announced_id; @@ -194,11 +193,9 @@ private: std::string last_print_state; // Track state for immediate dispatch on change // Connection thread management - std::atomic connect_in_progress{false}; - std::atomic connect_stop_requested{false}; - std::thread connect_thread; - std::recursive_mutex connect_mutex; - std::condition_variable connect_cv; + std::atomic connect_generation{0}; + std::thread connect_thread; + std::recursive_mutex connect_mutex; }; } // namespace Slic3r diff --git a/src/slic3r/Utils/NetworkAgent.cpp b/src/slic3r/Utils/NetworkAgent.cpp index 16d82ba8dc..1ec0ef437d 100644 --- a/src/slic3r/Utils/NetworkAgent.cpp +++ b/src/slic3r/Utils/NetworkAgent.cpp @@ -139,6 +139,7 @@ void NetworkAgent::set_printer_agent(std::shared_ptr printer_agen // Local copies to allow safe access after releasing the lock. // This pattern ensures the objects stay alive (via shared_ptr refcount) even if // another thread modifies m_printer_agent or m_printer_callbacks after we unlock. + std::shared_ptr old_printer_agent; std::shared_ptr new_printer_agent; PrinterCallbacks callbacks; @@ -152,6 +153,8 @@ void NetworkAgent::set_printer_agent(std::shared_ptr printer_agen // Disconnect all callbacks from the old agent apply_printer_callbacks(m_printer_agent, callbacks); + // Capture the old agent before overwriting so we can disconnect it outside the lock + old_printer_agent = m_printer_agent; // Take ownership of the incoming agent and update the agent ID m_printer_agent = std::move(printer_agent); m_printer_agent_id = m_printer_agent->get_agent_info().id; @@ -164,6 +167,11 @@ void NetworkAgent::set_printer_agent(std::shared_ptr printer_agen } // Lock released here - m_agent_mutex is now free for other threads + // Disconnect the old agent's connections/threads. The cache keeps it alive, + // but we release its network resources while it's not the active agent. + if (old_printer_agent && old_printer_agent != new_printer_agent) + old_printer_agent->disconnect_printer(); + // Apply callbacks OUTSIDE the lock to avoid deadlock risk and minimize // critical section duration. The local shared_ptr copy ensures the agent // cannot be destroyed while we're using it. diff --git a/src/slic3r/Utils/NetworkAgentFactory.cpp b/src/slic3r/Utils/NetworkAgentFactory.cpp index 025aec9bf0..4047155881 100644 --- a/src/slic3r/Utils/NetworkAgentFactory.cpp +++ b/src/slic3r/Utils/NetworkAgentFactory.cpp @@ -21,6 +21,12 @@ std::map& get_printer_agents() return agents; } +std::map>& get_printer_agent_cache() +{ + static std::map> cache; + return cache; +} + // Helper to register a printer agent type with the standard factory pattern. // AgentTypes that take a log_dir constructor arg use the default; BBLPrinterAgent // (no log_dir) is registered separately. @@ -82,15 +88,44 @@ std::shared_ptr NetworkAgentFactory::create_printer_agent_by_id(c const std::string& log_dir) { std::lock_guard lock(s_registry_mutex); - auto& agents = get_printer_agents(); - auto it = agents.find(id); + + // Check cache first + auto& cache = get_printer_agent_cache(); + auto cache_it = cache.find(id); + if (cache_it != cache.end()) { + BOOST_LOG_TRIVIAL(info) << "Reusing cached printer agent: " << id; + if (cloud_agent) + cache_it->second->set_cloud_agent(cloud_agent); + return cache_it->second; + } + + // Not cached — create via factory + auto& agents = get_printer_agents(); + auto it = agents.find(id); if (it == agents.end()) { BOOST_LOG_TRIVIAL(warning) << "Unknown printer agent ID: " << id; return nullptr; } - return it->second.factory(cloud_agent, log_dir); + auto agent = it->second.factory(cloud_agent, log_dir); + if (agent) { + BOOST_LOG_TRIVIAL(info) << "Created and cached printer agent: " << id; + cache[id] = agent; + } + return agent; +} + +void NetworkAgentFactory::clear_printer_agent_cache() +{ + std::lock_guard lock(s_registry_mutex); + auto& cache = get_printer_agent_cache(); + for (auto& pair : cache) { + if (pair.second) + pair.second->disconnect_printer(); + } + cache.clear(); + BOOST_LOG_TRIVIAL(info) << "Printer agent cache cleared"; } void NetworkAgentFactory::register_all_agents() diff --git a/src/slic3r/Utils/NetworkAgentFactory.hpp b/src/slic3r/Utils/NetworkAgentFactory.hpp index a4233ebc5f..56dd0efe9c 100644 --- a/src/slic3r/Utils/NetworkAgentFactory.hpp +++ b/src/slic3r/Utils/NetworkAgentFactory.hpp @@ -106,6 +106,9 @@ public: /** * Create a printer agent by ID (using registry) * + * Returns a cached instance if one exists for the given ID, otherwise + * creates a new agent via the registered factory and caches it. + * * @param id Agent ID to create * @param cloud_agent Cloud agent for token access * @param log_dir Directory for log files @@ -115,6 +118,13 @@ public: std::shared_ptr cloud_agent, const std::string& log_dir); + /** + * Clear the printer agent cache. + * Calls disconnect_printer() on each cached agent and releases all shared_ptrs. + * Should be called during application shutdown before destroying the NetworkAgent. + */ + static void clear_printer_agent_cache(); + // ======================================================================== // Cloud Agent Factory // ========================================================================