gronod/OpenTTD
1
0
Fork 0
mirror of https://github.com/OpenTTD/OpenTTD.git synced 2025-02-11 08:49:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix: Crash due to use of invalid iterator in ClientNetworkContentSocketHandler

Browse Source 
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
This commit is contained in:
Jonathan G Rennison 2019-04-08 07:57:42 +01:00 committed by Michael Lutz
parent 8b1880187a
commit 01f957c51f
1 changed files with 18 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/network/network_content.cpp
View File

@ -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++;
}
}