From 3d8249bffaf5b05d06fcbdcb806d99eeedae1731 Mon Sep 17 00:00:00 2001 From: dijunkun Date: Fri, 28 Nov 2025 00:41:29 +0800 Subject: [PATCH] [fix] use lock and null pointer checks to prevent crashes --- src/gui/panels/remote_peer_panel.cpp | 84 +++++++++++++++++----------- src/gui/render.cpp | 73 ++++++++++++++---------- src/gui/render.h | 3 + src/gui/render_callback.cpp | 36 ++++++++---- src/gui/windows/main_window.cpp | 1 + src/gui/windows/stream_window.cpp | 83 +++++++++++++++++++++------ 6 files changed, 189 insertions(+), 91 deletions(-) diff --git a/src/gui/panels/remote_peer_panel.cpp b/src/gui/panels/remote_peer_panel.cpp index d66c653..a1b66c8 100644 --- a/src/gui/panels/remote_peer_panel.cpp +++ b/src/gui/panels/remote_peer_panel.cpp @@ -82,6 +82,7 @@ int Render::RemoteWindow() { for (auto& [id, props] : recent_connections_) { if (id.find(remote_id) != std::string::npos) { found = true; + std::shared_lock lock(client_properties_mutex_); if (client_properties_.find(remote_id) != client_properties_.end()) { if (!client_properties_[remote_id]->connection_established_) { @@ -111,6 +112,7 @@ int Render::RemoteWindow() { if (elapsed >= 1000) { last_rejoin_check_time_ = now; need_to_rejoin_ = false; + std::shared_lock lock(client_properties_mutex_); for (const auto& [_, props] : client_properties_) { if (props->rejoin_) { ConnectTo(props->remote_id_, props->remote_password_, @@ -145,38 +147,49 @@ int Render::ConnectTo(const std::string& remote_id, const char* password, LOG_INFO("Connect to [{}]", remote_id); focused_remote_id_ = remote_id; - if (client_properties_.find(remote_id) == client_properties_.end()) { - client_properties_[remote_id] = - std::make_shared(); - auto props = client_properties_[remote_id]; - props->local_id_ = "C-" + std::string(client_id_); - props->remote_id_ = remote_id; - memcpy(&props->params_, ¶ms_, sizeof(Params)); - props->params_.user_id = props->local_id_.c_str(); - props->peer_ = CreatePeer(&props->params_); + std::shared_lock shared_lock(client_properties_mutex_); + bool exists = + (client_properties_.find(remote_id) != client_properties_.end()); + shared_lock.unlock(); - if (!props->peer_) { - LOG_INFO("Create peer [{}] instance failed", props->local_id_); - return -1; + if (!exists) { + std::unique_lock unique_lock(client_properties_mutex_); + if (client_properties_.find(remote_id) == client_properties_.end()) { + client_properties_[remote_id] = + std::make_shared(); + auto props = client_properties_[remote_id]; + props->local_id_ = "C-" + std::string(client_id_); + props->remote_id_ = remote_id; + memcpy(&props->params_, ¶ms_, sizeof(Params)); + props->params_.user_id = props->local_id_.c_str(); + props->peer_ = CreatePeer(&props->params_); + + if (!props->peer_) { + LOG_INFO("Create peer [{}] instance failed", props->local_id_); + return -1; + } + + for (auto& display_info : display_info_list_) { + AddVideoStream(props->peer_, display_info.name.c_str()); + } + AddAudioStream(props->peer_, props->audio_label_.c_str()); + AddDataStream(props->peer_, props->data_label_.c_str()); + + if (props->peer_) { + LOG_INFO("[{}] Create peer instance successful", props->local_id_); + Init(props->peer_); + LOG_INFO("[{}] Peer init finish", props->local_id_); + } else { + LOG_INFO("Create peer [{}] instance failed", props->local_id_); + } + + props->connection_status_ = ConnectionStatus::Connecting; } - - for (auto& display_info : display_info_list_) { - AddVideoStream(props->peer_, display_info.name.c_str()); - } - AddAudioStream(props->peer_, props->audio_label_.c_str()); - AddDataStream(props->peer_, props->data_label_.c_str()); - - if (props->peer_) { - LOG_INFO("[{}] Create peer instance successful", props->local_id_); - Init(props->peer_); - LOG_INFO("[{}] Peer init finish", props->local_id_); - } else { - LOG_INFO("Create peer [{}] instance failed", props->local_id_); - } - - props->connection_status_ = ConnectionStatus::Connecting; + unique_lock.unlock(); } + int ret = -1; + std::shared_lock read_lock(client_properties_mutex_); auto props = client_properties_[remote_id]; if (!props->connection_established_) { props->remember_password_ = remember_password; @@ -188,14 +201,17 @@ int Render::ConnectTo(const std::string& remote_id, const char* password, } std::string remote_id_with_pwd = remote_id + "@" + password; - ret = JoinConnection(props->peer_, remote_id_with_pwd.c_str()); - if (0 == ret) { - props->rejoin_ = false; - } else { - props->rejoin_ = true; - need_to_rejoin_ = true; + if (props->peer_) { + ret = JoinConnection(props->peer_, remote_id_with_pwd.c_str()); + if (0 == ret) { + props->rejoin_ = false; + } else { + props->rejoin_ = true; + need_to_rejoin_ = true; + } } } + read_lock.unlock(); return 0; } diff --git a/src/gui/render.cpp b/src/gui/render.cpp index 1ffc584..4815f97 100644 --- a/src/gui/render.cpp +++ b/src/gui/render.cpp @@ -947,6 +947,7 @@ int Render::DrawStreamWindow() { ImGui::Render(); SDL_RenderClear(stream_renderer_); + std::shared_lock lock(client_properties_mutex_); for (auto& it : client_properties_) { auto props = it.second; if (props->tab_selected_) { @@ -1283,12 +1284,18 @@ void Render::CleanupPeers() { DestroyPeer(&peer_); } - for (auto& it : client_properties_) { - auto props = it.second; - CleanupPeer(props); + { + std::shared_lock lock(client_properties_mutex_); + for (auto& it : client_properties_) { + auto props = it.second; + CleanupPeer(props); + } } - client_properties_.clear(); + { + std::unique_lock lock(client_properties_mutex_); + client_properties_.clear(); + } } void Render::CleanSubStreamWindowProperties( @@ -1305,6 +1312,7 @@ void Render::CleanSubStreamWindowProperties( } void Render::UpdateRenderRect() { + std::shared_lock lock(client_properties_mutex_); for (auto& [_, props] : client_properties_) { if (!props->reset_control_bar_pos_) { props->mouse_diff_control_bar_pos_x_ = 0; @@ -1379,34 +1387,41 @@ void Render::ProcessSdlEvent(const SDL_Event& event) { DestroyStreamWindow(); DestroyStreamWindowContext(); - for (auto& [host_name, props] : client_properties_) { - thumbnail_->SaveToThumbnail( - (char*)props->dst_buffer_, props->video_width_, - props->video_height_, host_name, props->remote_host_name_, - props->remember_password_ ? props->remote_password_ : ""); + { + std::shared_lock lock(client_properties_mutex_); + for (auto& [host_name, props] : client_properties_) { + thumbnail_->SaveToThumbnail( + (char*)props->dst_buffer_, props->video_width_, + props->video_height_, host_name, props->remote_host_name_, + props->remember_password_ ? props->remote_password_ : ""); - if (props->peer_) { - std::string client_id = (host_name == client_id_) - ? "C-" + std::string(client_id_) - : client_id_; - LOG_INFO("[{}] Leave connection [{}]", client_id, host_name); - LeaveConnection(props->peer_, host_name.c_str()); - LOG_INFO("Destroy peer [{}]", client_id); - DestroyPeer(&props->peer_); + if (props->peer_) { + std::string client_id = (host_name == client_id_) + ? "C-" + std::string(client_id_) + : client_id_; + LOG_INFO("[{}] Leave connection [{}]", client_id, host_name); + LeaveConnection(props->peer_, host_name.c_str()); + LOG_INFO("Destroy peer [{}]", client_id); + DestroyPeer(&props->peer_); + } + + props->streaming_ = false; + props->remember_password_ = false; + props->connection_established_ = false; + props->audio_capture_button_pressed_ = false; + + memset(&props->net_traffic_stats_, 0, + sizeof(props->net_traffic_stats_)); + SDL_SetWindowFullscreen(main_window_, false); + SDL_FlushEvents(STREAM_REFRESH_EVENT, STREAM_REFRESH_EVENT); + memset(audio_buffer_, 0, 720); } - - props->streaming_ = false; - props->remember_password_ = false; - props->connection_established_ = false; - props->audio_capture_button_pressed_ = false; - - memset(&props->net_traffic_stats_, 0, - sizeof(props->net_traffic_stats_)); - SDL_SetWindowFullscreen(main_window_, false); - SDL_FlushEvents(STREAM_REFRESH_EVENT, STREAM_REFRESH_EVENT); - memset(audio_buffer_, 0, 720); } - client_properties_.clear(); + + { + std::unique_lock lock(client_properties_mutex_); + client_properties_.clear(); + } rejoin_ = false; is_client_mode_ = false; diff --git a/src/gui/render.h b/src/gui/render.h index 9063f6a..1e934a8 100644 --- a/src/gui/render.h +++ b/src/gui/render.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -30,6 +31,7 @@ #include "screen_capturer_factory.h" #include "speaker_capturer_factory.h" #include "thumbnail.h" + #if _WIN32 #include "win_tray.h" #endif @@ -504,6 +506,7 @@ class Render { /* ------ sub stream window property start ------ */ std::unordered_map> client_properties_; + std::shared_mutex client_properties_mutex_; void CloseTab(decltype(client_properties_)::iterator& it); /* ------ stream window property end ------ */ diff --git a/src/gui/render_callback.cpp b/src/gui/render_callback.cpp index 34eef0c..924cd6b 100644 --- a/src/gui/render_callback.cpp +++ b/src/gui/render_callback.cpp @@ -21,13 +21,16 @@ int Render::SendKeyCommand(int key_code, bool is_down) { remote_action.k.key_value = key_code; if (!controlled_remote_id_.empty()) { + std::shared_lock lock(client_properties_mutex_); if (client_properties_.find(controlled_remote_id_) != client_properties_.end()) { auto props = client_properties_[controlled_remote_id_]; if (props->connection_status_ == ConnectionStatus::Connected) { std::string msg = remote_action.to_json(); - SendDataFrame(props->peer_, msg.c_str(), msg.size(), - props->data_label_.c_str()); + if (props->peer_) { + SendDataFrame(props->peer_, msg.c_str(), msg.size(), + props->data_label_.c_str()); + } } } } @@ -42,6 +45,7 @@ int Render::ProcessMouseEvent(const SDL_Event& event) { float ratio_x, ratio_y = 0; RemoteAction remote_action; + std::shared_lock lock(client_properties_mutex_); for (auto& it : client_properties_) { auto props = it.second; if (!props->control_mouse_) { @@ -94,8 +98,10 @@ int Render::ProcessMouseEvent(const SDL_Event& event) { } std::string msg = remote_action.to_json(); - SendDataFrame(props->peer_, msg.c_str(), msg.size(), - props->data_label_.c_str()); + if (props->peer_) { + SendDataFrame(props->peer_, msg.c_str(), msg.size(), + props->data_label_.c_str()); + } } else if (SDL_EVENT_MOUSE_WHEEL == event.type && last_mouse_event.button.x >= props->stream_render_rect_.x && last_mouse_event.button.x <= props->stream_render_rect_.x + @@ -139,8 +145,10 @@ int Render::ProcessMouseEvent(const SDL_Event& event) { render_height; std::string msg = remote_action.to_json(); - SendDataFrame(props->peer_, msg.c_str(), msg.size(), - props->data_label_.c_str()); + if (props->peer_) { + SendDataFrame(props->peer_, msg.c_str(), msg.size(), + props->data_label_.c_str()); + } } } @@ -154,11 +162,14 @@ void Render::SdlCaptureAudioIn(void* userdata, Uint8* stream, int len) { } if (1) { - for (auto it : render->client_properties_) { + std::shared_lock lock(render->client_properties_mutex_); + for (const auto& it : render->client_properties_) { auto props = it.second; if (props->connection_status_ == ConnectionStatus::Connected) { - SendAudioFrame(props->peer_, (const char*)stream, len, - render->audio_label_.c_str()); + if (props->peer_) { + SendAudioFrame(props->peer_, (const char*)stream, len, + render->audio_label_.c_str()); + } } } @@ -207,6 +218,7 @@ void Render::OnReceiveVideoBufferCb(const XVideoFrame* video_frame, } std::string remote_id(user_id, user_id_size); + std::shared_lock lock(render->client_properties_mutex_); if (render->client_properties_.find(remote_id) == render->client_properties_.end()) { return; @@ -302,6 +314,7 @@ void Render::OnReceiveDataBufferCb(const char* data, size_t size, } std::string remote_id(user_id, user_id_size); + std::shared_lock lock(render->client_properties_mutex_); if (render->client_properties_.find(remote_id) != render->client_properties_.end()) { // local @@ -373,6 +386,7 @@ void Render::OnSignalStatusCb(SignalStatus status, const char* user_id, } std::string remote_id(client_id.begin() + 2, client_id.end()); + std::shared_lock lock(render->client_properties_mutex_); if (render->client_properties_.find(remote_id) == render->client_properties_.end()) { return; @@ -402,6 +416,7 @@ void Render::OnConnectionStatusCb(ConnectionStatus status, const char* user_id, if (!render) return; std::string remote_id(user_id, user_id_size); + std::shared_lock lock(render->client_properties_mutex_); auto it = render->client_properties_.find(remote_id); auto props = (it != render->client_properties_.end()) ? it->second : nullptr; @@ -428,7 +443,7 @@ void Render::OnConnectionStatusCb(ConnectionStatus status, const char* user_id, case ConnectionStatus::Closed: { props->connection_established_ = false; props->mouse_control_button_pressed_ = false; - if (props->dst_buffer_) { + if (props->dst_buffer_ && props->stream_texture_) { memset(props->dst_buffer_, 0, props->dst_buffer_capacity_); SDL_UpdateTexture(props->stream_texture_, NULL, props->dst_buffer_, props->texture_width_); @@ -562,6 +577,7 @@ void Render::NetStatusReport(const char* client_id, size_t client_id_size, } std::string remote_id(user_id, user_id_size); + std::shared_lock lock(render->client_properties_mutex_); if (render->client_properties_.find(remote_id) == render->client_properties_.end()) { return; diff --git a/src/gui/windows/main_window.cpp b/src/gui/windows/main_window.cpp index a810398..2c158e5 100644 --- a/src/gui/windows/main_window.cpp +++ b/src/gui/windows/main_window.cpp @@ -32,6 +32,7 @@ int Render::MainWindow() { StatusBar(); if (show_connection_status_window_) { + std::unique_lock lock(client_properties_mutex_); for (auto it = client_properties_.begin(); it != client_properties_.end();) { auto& props = it->second; diff --git a/src/gui/windows/stream_window.cpp b/src/gui/windows/stream_window.cpp index 705f015..937bf6a 100644 --- a/src/gui/windows/stream_window.cpp +++ b/src/gui/windows/stream_window.cpp @@ -32,12 +32,15 @@ void Render::DrawConnectionStatusText( } void Render::CloseTab(decltype(client_properties_)::iterator& it) { - CleanupPeer(it->second); - it = client_properties_.erase(it); - if (client_properties_.empty()) { - SDL_Event event; - event.type = SDL_EVENT_QUIT; - SDL_PushEvent(&event); + std::unique_lock lock(client_properties_mutex_); + if (it != client_properties_.end()) { + CleanupPeer(it->second); + it = client_properties_.erase(it); + if (client_properties_.empty()) { + SDL_Event event; + event.type = SDL_EVENT_QUIT; + SDL_PushEvent(&event); + } } } @@ -79,11 +82,22 @@ int Render::StreamWindow() { ImGuiTabBarFlags_AutoSelectNewTabs)) { is_tab_bar_hovered_ = ImGui::IsWindowHovered(); + std::shared_lock lock(client_properties_mutex_); for (auto it = client_properties_.begin(); it != client_properties_.end();) { auto& props = it->second; if (!props->tab_opened_) { - CloseTab(it); + std::string remote_id_to_close = props->remote_id_; + lock.unlock(); + { + std::unique_lock unique_lock(client_properties_mutex_); + auto close_it = client_properties_.find(remote_id_to_close); + if (close_it != client_properties_.end()) { + CloseTab(close_it); + } + } + lock.lock(); + it = client_properties_.begin(); continue; } @@ -122,12 +136,23 @@ int Render::StreamWindow() { focused_remote_id_ = props->remote_id_; if (!props->peer_) { - it = client_properties_.erase(it); - if (client_properties_.empty()) { - SDL_Event event; - event.type = SDL_EVENT_QUIT; - SDL_PushEvent(&event); + std::string remote_id_to_erase = props->remote_id_; + lock.unlock(); + { + std::unique_lock unique_lock(client_properties_mutex_); + auto erase_it = client_properties_.find(remote_id_to_erase); + if (erase_it != client_properties_.end()) { + erase_it = client_properties_.erase(erase_it); + if (client_properties_.empty()) { + SDL_Event event; + event.type = SDL_EVENT_QUIT; + SDL_PushEvent(&event); + } + } } + lock.lock(); + it = client_properties_.begin(); + continue; } else { DrawConnectionStatusText(props); ++it; @@ -147,11 +172,22 @@ int Render::StreamWindow() { ImGui::End(); // End TabBar } else { + std::shared_lock lock(client_properties_mutex_); for (auto it = client_properties_.begin(); it != client_properties_.end();) { auto& props = it->second; if (!props->tab_opened_) { - CloseTab(it); + std::string remote_id_to_close = props->remote_id_; + lock.unlock(); + { + std::unique_lock unique_lock(client_properties_mutex_); + auto close_it = client_properties_.find(remote_id_to_close); + if (close_it != client_properties_.end()) { + CloseTab(close_it); + } + } + lock.lock(); + it = client_properties_.begin(); continue; } @@ -181,12 +217,23 @@ int Render::StreamWindow() { if (!props->peer_) { fullscreen_button_pressed_ = false; SDL_SetWindowFullscreen(stream_window_, false); - it = client_properties_.erase(it); - if (client_properties_.empty()) { - SDL_Event event; - event.type = SDL_EVENT_QUIT; - SDL_PushEvent(&event); + std::string remote_id_to_erase = props->remote_id_; + lock.unlock(); + { + std::unique_lock unique_lock(client_properties_mutex_); + auto erase_it = client_properties_.find(remote_id_to_erase); + if (erase_it != client_properties_.end()) { + client_properties_.erase(erase_it); + if (client_properties_.empty()) { + SDL_Event event; + event.type = SDL_EVENT_QUIT; + SDL_PushEvent(&event); + } + } } + lock.lock(); + it = client_properties_.begin(); + continue; } else { DrawConnectionStatusText(props); ++it;