diff --git a/NetworkClient.cpp b/NetworkClient.cpp index 88ddcb175..cb6885a2f 100644 --- a/NetworkClient.cpp +++ b/NetworkClient.cpp @@ -106,6 +106,20 @@ NetworkClient::NetworkClient() NetworkClient::~NetworkClient() { StopClient(); + + /*-----------------------------------------------------*\ + | Free the controllers set aside by StopClient, plus | + | any left behind by a listener that never ran | + \*-----------------------------------------------------*/ + ControllerListMutex.lock(); + + orphaned_controllers.insert(orphaned_controllers.end(), server_controllers.begin(), server_controllers.end()); + + server_controllers.clear(); + + ControllerListMutex.unlock(); + + DeleteOrphanedControllers(); } /*---------------------------------------------------------*\ @@ -326,6 +340,36 @@ void NetworkClient::StopClient() SignalNetworkClientUpdate(NETWORKCLIENT_UPDATE_REASON_CLIENT_STOPPED); } +/*---------------------------------------------------------*\ +| Controller teardown functions | +\*---------------------------------------------------------*/ +void NetworkClient::TakeOrphanedControllers(std::vector& orphaned) +{ + ControllerListMutex.lock(); + + orphaned.insert(orphaned.end(), orphaned_controllers.begin(), orphaned_controllers.end()); + + orphaned_controllers.clear(); + + ControllerListMutex.unlock(); +} + +void NetworkClient::DeleteOrphanedControllers() +{ + /*-----------------------------------------------------*\ + | Delete outside the mutex; the controller destructor | + | waits for callbacks that are already running | + \*-----------------------------------------------------*/ + std::vector orphaned; + + TakeOrphanedControllers(orphaned); + + for(std::size_t orphaned_idx = 0; orphaned_idx < orphaned.size(); orphaned_idx++) + { + delete orphaned[orphaned_idx]; + } +} + void NetworkClient::SendRequest_ControllerData(unsigned int dev_id) { NetPacketHeader request_hdr; @@ -1905,23 +1949,27 @@ listen_done: server_initialized = false; server_connected = false; + /*-----------------------------------------------------*\ + | Do not delete the controllers here; the front end | + | still holds pointers to them. Move them to | + | orphaned_controllers for the teardown thread to free | + \*-----------------------------------------------------*/ ControllerListMutex.lock(); - std::vector server_controllers_copy = server_controllers; + orphaned_controllers.insert(orphaned_controllers.end(), server_controllers.begin(), server_controllers.end()); server_controllers.clear(); - for(size_t server_controller_idx = 0; server_controller_idx < server_controllers_copy.size(); server_controller_idx++) - { - delete server_controllers_copy[server_controller_idx]; - } - ControllerListMutex.unlock(); /*-----------------------------------------------------*\ - | Client info has changed, call the callbacks | + | Signal only an unplanned connection loss; on a | + | requested stop the caller runs the teardown | \*-----------------------------------------------------*/ - SignalNetworkClientUpdate(NETWORKCLIENT_UPDATE_REASON_CLIENT_DISCONNECTED); + if(client_active) + { + SignalNetworkClientUpdate(NETWORKCLIENT_UPDATE_REASON_CLIENT_DISCONNECTED); + } } /*---------------------------------------------------------*\ diff --git a/NetworkClient.h b/NetworkClient.h index f03926892..cd4c32979 100644 --- a/NetworkClient.h +++ b/NetworkClient.h @@ -100,6 +100,12 @@ public: void StartClient(); void StopClient(); + /*-----------------------------------------------------*\ + | Controller teardown functions | + \*-----------------------------------------------------*/ + void DeleteOrphanedControllers(); + void TakeOrphanedControllers(std::vector& orphaned); + void SendRequest_ControllerData(unsigned int dev_id); void SendRequest_RescanDevices(); @@ -249,6 +255,7 @@ private: \*-----------------------------------------------------*/ std::mutex ControllerListMutex; std::vector server_controllers; + std::vector orphaned_controllers; std::vector server_controller_ids; /*-----------------------------------------------------*\ diff --git a/ResourceManager.cpp b/ResourceManager.cpp index 83fa7aaaa..19bc614bc 100644 --- a/ResourceManager.cpp +++ b/ResourceManager.cpp @@ -86,6 +86,19 @@ static void ResourceManagerNetworkClientCallback(void* this_ptr, unsigned int up this_obj->UpdateDeviceList(); break; + case NETWORKCLIENT_UPDATE_REASON_CLIENT_CONNECTED: + this_obj->SignalResourceManagerUpdate(RESOURCEMANAGER_UPDATE_REASON_CLIENT_INFO_UPDATED); + break; + + /*-------------------------------------------------*\ + | Unplanned connection loss: free the client's | + | orphaned controllers on the teardown thread | + \*-------------------------------------------------*/ + case NETWORKCLIENT_UPDATE_REASON_CLIENT_DISCONNECTED: + this_obj->SignalResourceManagerUpdate(RESOURCEMANAGER_UPDATE_REASON_CLIENT_INFO_UPDATED); + this_obj->QueueClientTeardown(); + break; + case NETWORKCLIENT_UPDATE_REASON_DETECTION_STARTED: this_obj->SignalResourceManagerUpdate(RESOURCEMANAGER_UPDATE_REASON_DETECTION_STARTED); break; @@ -151,6 +164,13 @@ ResourceManager::ResourceManager() plugin_manager = NULL; server = NULL; + /*-----------------------------------------------------*\ + | Start the client teardown thread | + \*-----------------------------------------------------*/ + ClientTeardownPending = false; + ClientTeardownRunning = true; + ClientTeardownThread = new std::thread(&ResourceManager::ClientTeardownThreadFunction, this); + SetupConfigurationDirectory(); /*-----------------------------------------------------*\ @@ -274,7 +294,21 @@ ResourceManager::ResourceManager() ResourceManager::~ResourceManager() { + /*-----------------------------------------------------*\ + | Stop the client teardown thread | + \*-----------------------------------------------------*/ + ClientTeardownMutex.lock(); + ClientTeardownRunning = false; + ClientTeardownMutex.unlock(); + ClientTeardownCv.notify_all(); + + if(ClientTeardownThread) + { + ClientTeardownThread->join(); + delete ClientTeardownThread; + ClientTeardownThread = nullptr; + } } /*---------------------------------------------------------*\ @@ -535,38 +569,142 @@ void ResourceManager::RegisterNetworkClient(NetworkClient* new_client) { new_client->RegisterNetworkClientCallback(ResourceManagerNetworkClientCallback, this); + ClientListMutex.lock(); + clients.push_back(new_client); + + ClientListMutex.unlock(); } void ResourceManager::UnregisterNetworkClient(NetworkClient* network_client) { /*-----------------------------------------------------*\ - | Stop the disconnecting client | + | Queue the removal for the teardown thread. This is | + | called on the GUI thread, and the teardown rebuilds | + | the device list, whose DEVICE_LIST_UPDATED handler | + | is a BlockingQueuedConnection to the GUI thread. It | + | also keeps StopClient's join off the GUI thread | \*-----------------------------------------------------*/ - network_client->StopClient(); + ClientTeardownMutex.lock(); - /*-----------------------------------------------------*\ - | Clear callbacks from the client before removal | - \*-----------------------------------------------------*/ - network_client->ClearCallbacks(); - - /*-----------------------------------------------------*\ - | Find the client to remove and remove it from the | - | clients list | - \*-----------------------------------------------------*/ - std::vector::iterator client_it = std::find(clients.begin(), clients.end(), network_client); - - if(client_it != clients.end()) + if(std::find(clients_to_remove.begin(), clients_to_remove.end(), network_client) == clients_to_remove.end()) { - clients.erase(client_it); + clients_to_remove.push_back(network_client); } - /*-----------------------------------------------------*\ - | Delete the client | - \*-----------------------------------------------------*/ - delete network_client; + ClientTeardownPending = true; - UpdateDeviceList(); + ClientTeardownMutex.unlock(); + + ClientTeardownCv.notify_all(); +} + +void ResourceManager::QueueClientTeardown() +{ + ClientTeardownMutex.lock(); + ClientTeardownPending = true; + ClientTeardownMutex.unlock(); + + ClientTeardownCv.notify_all(); +} + +void ResourceManager::ClientTeardownThreadFunction() +{ + std::unique_lock lock(ClientTeardownMutex); + + while(ClientTeardownRunning) + { + if(!ClientTeardownPending) + { + ClientTeardownCv.wait(lock); + continue; + } + + ClientTeardownPending = false; + + /*-------------------------------------------------*\ + | Take the clients queued for removal off the | + | pending list | + \*-------------------------------------------------*/ + std::vector remove_list; + + remove_list.swap(clients_to_remove); + + lock.unlock(); + + /*-------------------------------------------------*\ + | Stop the clients being removed and take them out | + | of the clients list. StopClient joins the client | + | threads and moves the client's controllers to | + | orphaned_controllers; the delete below frees them | + \*-------------------------------------------------*/ + for(std::size_t remove_idx = 0; remove_idx < remove_list.size(); remove_idx++) + { + remove_list[remove_idx]->StopClient(); + remove_list[remove_idx]->ClearCallbacks(); + + ClientListMutex.lock(); + + std::vector::iterator client_it = std::find(clients.begin(), clients.end(), remove_list[remove_idx]); + + if(client_it != clients.end()) + { + clients.erase(client_it); + } + + ClientListMutex.unlock(); + } + + /*-------------------------------------------------*\ + | Collect the orphaned controllers from the clients | + | that remain. Ownership moves here, so a client | + | deleted later cannot free them a second time | + \*-------------------------------------------------*/ + std::vector orphaned; + + ClientListMutex.lock(); + + for(std::size_t client_idx = 0; client_idx < clients.size(); client_idx++) + { + clients[client_idx]->TakeOrphanedControllers(orphaned); + } + + ClientListMutex.unlock(); + + /*-------------------------------------------------*\ + | Run the same sequence a rescan does while the | + | controllers are still allocated. Consumers | + | release them on DETECTION_STARTED, the GUI | + | deletes the device pages (whose destructors use | + | the controller) on UpdateDeviceList, and plugins | + | rebuild on DETECTION_COMPLETE. Free them last, | + | once nothing references them | + \*-------------------------------------------------*/ + if(!remove_list.empty() || !orphaned.empty()) + { + SignalResourceManagerUpdate(RESOURCEMANAGER_UPDATE_REASON_DETECTION_STARTED); + + UpdateDeviceList(); + + SignalResourceManagerUpdate(RESOURCEMANAGER_UPDATE_REASON_DETECTION_COMPLETE); + + /*---------------------------------------------*\ + | Deleting a removed client frees the | + | controllers StopClient left on it | + \*---------------------------------------------*/ + for(std::size_t remove_idx = 0; remove_idx < remove_list.size(); remove_idx++) + { + delete remove_list[remove_idx]; + } + + for(std::size_t orphaned_idx = 0; orphaned_idx < orphaned.size(); orphaned_idx++) + { + delete orphaned[orphaned_idx]; + } + } + + lock.lock(); + } } /*---------------------------------------------------------*\ @@ -742,6 +880,8 @@ void ResourceManager::UpdateDeviceList() /*-----------------------------------------------------*\ | Insert client controllers into controller list | \*-----------------------------------------------------*/ + ClientListMutex.lock(); + for(std::size_t client_idx = 0; client_idx < clients.size(); client_idx++) { std::vector rgb_controllers_client = clients[client_idx]->GetRGBControllers(); @@ -752,6 +892,8 @@ void ResourceManager::UpdateDeviceList() } } + ClientListMutex.unlock(); + /*-----------------------------------------------------*\ | Update server list | \*-----------------------------------------------------*/ diff --git a/ResourceManager.h b/ResourceManager.h index 62da96cdb..cf3c8cae7 100644 --- a/ResourceManager.h +++ b/ResourceManager.h @@ -13,7 +13,9 @@ #pragma once +#include #include +#include #include #include #include @@ -102,6 +104,7 @@ public: \*-----------------------------------------------------*/ void RegisterNetworkClient(NetworkClient* new_client); void UnregisterNetworkClient(NetworkClient* network_client); + void QueueClientTeardown(); /*-----------------------------------------------------*\ | Local Client Accessors | @@ -139,6 +142,7 @@ public: private: bool AttemptLocalConnection(); + void ClientTeardownThreadFunction(); void SetupConfigurationDirectory(); /*-----------------------------------------------------*\ @@ -214,6 +218,23 @@ private: | Network Clients | \*-----------------------------------------------------*/ std::vector clients; + std::mutex ClientListMutex; + + /*-----------------------------------------------------*\ + | Client teardown thread | + | | + | Frees controllers a lost connection left behind, | + | inside a DETECTION_STARTED / DETECTION_COMPLETE | + | sequence. Runs on its own thread: StopClient joins | + | the client's listener thread, and the sequence's | + | callbacks need the GUI thread free | + \*-----------------------------------------------------*/ + std::thread* ClientTeardownThread; + std::mutex ClientTeardownMutex; + std::condition_variable ClientTeardownCv; + bool ClientTeardownPending; + bool ClientTeardownRunning; + std::vector clients_to_remove; /*-----------------------------------------------------*\ | Device List Mutex |