From d26abb724e4d64ca7be507d99d80ab778813684f Mon Sep 17 00:00:00 2001 From: Bjørn Lindeijer Date: Fri, 5 Dec 2008 21:38:31 +0100 Subject: Fix race condition with a std::string access The downloading thread was writing to a std::string while the main thread was trying to draw it, for example. Now access to the label caption is guarded with a mutex. Should fix crashes while downloading updates. (cherry picked from eAthena branch, commits 6ac9c3bce62a8fc79e23477417188108f0ad9fa6 and 06d0205bab253ec5d01e8483ab639a092fe117c5) --- src/gui/updatewindow.cpp | 31 +++++++++++++++++++------------ src/gui/updatewindow.h | 13 ++++++++++--- 2 files changed, 29 insertions(+), 15 deletions(-) (limited to 'src/gui') diff --git a/src/gui/updatewindow.cpp b/src/gui/updatewindow.cpp index 0d6b00af..997a9b82 100644 --- a/src/gui/updatewindow.cpp +++ b/src/gui/updatewindow.cpp @@ -47,7 +47,7 @@ /** * Calculates the Alder-32 checksum for the given file. */ -unsigned long fadler32(FILE *file) +static unsigned long fadler32(FILE *file) { // Obtain file size fseek(file, 0, SEEK_END); @@ -146,15 +146,9 @@ UpdaterWindow::UpdaterWindow(const std::string &updateHost, UpdaterWindow::~UpdaterWindow() { if (mThread) - { - SDL_WaitThread(mThread, NULL); - mThread = NULL; - } + SDL_WaitThread(mThread, NULL); - if (mMemoryBuffer) - { - free(mMemoryBuffer); - } + free(mMemoryBuffer); // Remove possibly leftover temporary download ::remove((mUpdatesDir + "/download.temp").c_str()); @@ -169,8 +163,9 @@ void UpdaterWindow::setProgress(float p) void UpdaterWindow::setLabel(const std::string &str) { - mLabel->setCaption(str); - mLabel->adjustSize(); + // Do delayed label text update, since Guichan isn't thread-safe + MutexLocker lock(&mLabelMutex); + mNewLabelCaption = str; } void UpdaterWindow::enable() @@ -429,6 +424,17 @@ void UpdaterWindow::logic() // Update Scroll logic mScrollArea->logic(); + // Synchronize label caption when necessary + { + MutexLocker lock(&mLabelMutex); + + if (mLabel->getCaption() != mNewLabelCaption) + { + mLabel->setCaption(mNewLabelCaption); + mLabel->adjustSize(); + } + } + switch (mDownloadStatus) { case UPDATE_ERROR: @@ -450,7 +456,8 @@ void UpdaterWindow::logic() mBrowserBox->addRow("##1 It is strongly recommended that"); mBrowserBox->addRow("##1 you try again later"); mBrowserBox->addRow(mCurlError); - mScrollArea->setVerticalScrollAmount(mScrollArea->getVerticalMaxScroll()); + mScrollArea->setVerticalScrollAmount( + mScrollArea->getVerticalMaxScroll()); mDownloadStatus = UPDATE_COMPLETE; break; case UPDATE_NEWS: diff --git a/src/gui/updatewindow.h b/src/gui/updatewindow.h index d7e3c4c7..a7dfe2cb 100644 --- a/src/gui/updatewindow.h +++ b/src/gui/updatewindow.h @@ -30,12 +30,13 @@ #include "../guichanfwd.h" +#include "../utils/mutex.h" + class BrowserBox; class Button; class ProgressBar; class ScrollArea; -struct SDL_mutex; struct SDL_Thread; /** @@ -88,7 +89,7 @@ class UpdaterWindow : public Window, public gcn::ActionListener int updateState; - protected: +private: void download(); /** @@ -133,6 +134,12 @@ class UpdaterWindow : public Window, public gcn::ActionListener /** The file currently downloading. */ std::string mCurrentFile; + /** The new label caption to be set in the logic method. */ + std::string mNewLabelCaption; + + /** The mutex used to guard access to mNewLabelCaption. */ + Mutex mLabelMutex; + /** The Adler32 checksum of the file currently downloading. */ unsigned long mCurrentChecksum; @@ -164,7 +171,7 @@ class UpdaterWindow : public Window, public gcn::ActionListener Button *mCancelButton; /**< Button to stop the update process. */ Button *mPlayButton; /**< Button to start playing. */ ProgressBar *mProgressBar; /**< Update progress bar. */ - BrowserBox* mBrowserBox; /**< Box to display news. */ + BrowserBox *mBrowserBox; /**< Box to display news. */ ScrollArea *mScrollArea; /**< Used to scroll news box. */ }; -- cgit v1.2.3-70-g09d2