From 01f957c51fcb294c4adb178fc6a1a123fe7c0d92 Mon Sep 17 00:00:00 2001 From: Jonathan G Rennison Date: Mon, 8 Apr 2019 07:57:42 +0100 Subject: Fix: Crash due to use of invalid iterator in ClientNetworkContentSocketHandler In particular this crash can be observed when using the bootstrap GUI to download the base graphics. In ClientNetworkContentSocketHandler::OnReceiveContentInfo ClientNetworkContentSocketHandler::callbacks is iterated, using an iterator cb->OnReceiveContentInfo() is called (cb is of type BootstrapAskForDownloadWindow) This calls new BootstrapContentDownloadStatusWindow() This inherits from BaseNetworkContentDownloadStatusWindow The constructor of which calls _network_content_client.AddCallback(this) This reallocates the std::vector which is being iterated in ClientNetworkContentSocketHandler::OnReceiveContentInfo This results in iter being invalid, and an assertion failure occurs shortly afterwards due to its use in the next iteration of cb->OnReceiveContentInfo() Adjust all locations where ClientNetworkContentSocketHandler::callbacks is iterated to avoid problematic behaviour --- src/network/network_content.cpp | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) (limited to 'src/network') diff --git a/src/network/network_content.cpp b/src/network/network_content.cpp index 19235c8c7..168ecd429 100644 --- a/src/network/network_content.cpp +++ b/src/network/network_content.cpp @@ -1029,37 +1029,39 @@ void ClientNetworkContentSocketHandler::Clear() void ClientNetworkContentSocketHandler::OnConnect(bool success) { - for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) { - ContentCallback *cb = *iter; + for (size_t i = 0; i < this->callbacks.size(); /* nothing */) { + ContentCallback *cb = this->callbacks[i]; + /* the callback may remove itself from this->callbacks */ cb->OnConnect(success); - if (iter != this->callbacks.end() && *iter == cb) iter++; + if (i != this->callbacks.size() && this->callbacks[i] == cb) i++; } } void ClientNetworkContentSocketHandler::OnDisconnect() { - for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) { - ContentCallback *cb = *iter; + for (size_t i = 0; i < this->callbacks.size(); /* nothing */) { + ContentCallback *cb = this->callbacks[i]; cb->OnDisconnect(); - if (iter != this->callbacks.end() && *iter == cb) iter++; + if (i != this->callbacks.size() && this->callbacks[i] == cb) i++; } } void ClientNetworkContentSocketHandler::OnReceiveContentInfo(const ContentInfo *ci) { - for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) { - ContentCallback *cb = *iter; + for (size_t i = 0; i < this->callbacks.size(); /* nothing */) { + ContentCallback *cb = this->callbacks[i]; + /* the callback may add items and/or remove itself from this->callbacks */ cb->OnReceiveContentInfo(ci); - if (iter != this->callbacks.end() && *iter == cb) iter++; + if (i != this->callbacks.size() && this->callbacks[i] == cb) i++; } } void ClientNetworkContentSocketHandler::OnDownloadProgress(const ContentInfo *ci, int bytes) { - for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) { - ContentCallback *cb = *iter; + for (size_t i = 0; i < this->callbacks.size(); /* nothing */) { + ContentCallback *cb = this->callbacks[i]; cb->OnDownloadProgress(ci, bytes); - if (iter != this->callbacks.end() && *iter == cb) iter++; + if (i != this->callbacks.size() && this->callbacks[i] == cb) i++; } } @@ -1070,9 +1072,10 @@ void ClientNetworkContentSocketHandler::OnDownloadComplete(ContentID cid) ci->state = ContentInfo::ALREADY_HERE; } - for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) { - ContentCallback *cb = *iter; + for (size_t i = 0; i < this->callbacks.size(); /* nothing */) { + ContentCallback *cb = this->callbacks[i]; + /* the callback may remove itself from this->callbacks */ cb->OnDownloadComplete(cid); - if (iter != this->callbacks.end() && *iter == cb) iter++; + if (i != this->callbacks.size() && this->callbacks[i] == cb) i++; } } -- cgit v1.2.3-70-g09d2