Commit e27053fae607ff8c923f180ee0ee06e8d780ea08
1 parent
291b1894
Prevent accidental deletion of wrong client in removeQueuedClients
No issues were reported, but I noticed the theoretical issue.
Showing
1 changed file
with
5 additions
and
5 deletions
threaddata.cpp
| @@ -245,8 +245,8 @@ void ThreadData::sendAllDisconnects() | @@ -245,8 +245,8 @@ void ThreadData::sendAllDisconnects() | ||
| 245 | 245 | ||
| 246 | void ThreadData::removeQueuedClients() | 246 | void ThreadData::removeQueuedClients() |
| 247 | { | 247 | { |
| 248 | - std::vector<int> fds; | ||
| 249 | - fds.reserve(1024); // 1024 is arbitrary... | 248 | + // Using shared pointers to have a claiming reference in case we lose the clients between the two locks. |
| 249 | + std::vector<std::shared_ptr<Client>> clients; | ||
| 250 | 250 | ||
| 251 | { | 251 | { |
| 252 | std::lock_guard<std::mutex> lck2(clientsToRemoveMutex); | 252 | std::lock_guard<std::mutex> lck2(clientsToRemoveMutex); |
| @@ -256,8 +256,7 @@ void ThreadData::removeQueuedClients() | @@ -256,8 +256,7 @@ void ThreadData::removeQueuedClients() | ||
| 256 | std::shared_ptr<Client> client = c.lock(); | 256 | std::shared_ptr<Client> client = c.lock(); |
| 257 | if (client) | 257 | if (client) |
| 258 | { | 258 | { |
| 259 | - int fd = client->getFd(); | ||
| 260 | - fds.push_back(fd); | 259 | + clients.push_back(client); |
| 261 | } | 260 | } |
| 262 | } | 261 | } |
| 263 | 262 | ||
| @@ -266,8 +265,9 @@ void ThreadData::removeQueuedClients() | @@ -266,8 +265,9 @@ void ThreadData::removeQueuedClients() | ||
| 266 | 265 | ||
| 267 | { | 266 | { |
| 268 | std::lock_guard<std::mutex> lck(clients_by_fd_mutex); | 267 | std::lock_guard<std::mutex> lck(clients_by_fd_mutex); |
| 269 | - for(int fd : fds) | 268 | + for(const std::shared_ptr<Client> &client : clients) |
| 270 | { | 269 | { |
| 270 | + int fd = client->getFd(); | ||
| 271 | clients_by_fd.erase(fd); | 271 | clients_by_fd.erase(fd); |
| 272 | } | 272 | } |
| 273 | } | 273 | } |