summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBjørn Lindeijer <bjorn@lindeijer.nl>2008-12-05 21:38:31 +0100
committerBjørn Lindeijer <bjorn@lindeijer.nl>2008-12-07 01:49:54 +0100
commitd26abb724e4d64ca7be507d99d80ab778813684f (patch)
tree260f00ec675b38ea69ba085eaa7a8d61af2bc32b
parentc4680eb890064cd2491dfc578941c72fe1463a82 (diff)
downloadmana-client-d26abb724e4d64ca7be507d99d80ab778813684f.tar.gz
mana-client-d26abb724e4d64ca7be507d99d80ab778813684f.tar.bz2
mana-client-d26abb724e4d64ca7be507d99d80ab778813684f.tar.xz
mana-client-d26abb724e4d64ca7be507d99d80ab778813684f.zip
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)
-rw-r--r--src/Makefile.am1
-rw-r--r--src/gui/updatewindow.cpp31
-rw-r--r--src/gui/updatewindow.h13
-rw-r--r--src/utils/mutex.h97
4 files changed, 127 insertions, 15 deletions
diff --git a/src/Makefile.am b/src/Makefile.am
index e953507a..65e8cafd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -282,6 +282,7 @@ tmw_SOURCES = gui/widgets/avatar.cpp \
utils/strprintf.cpp \
utils/tostring.h \
utils/trim.h \
+ utils/mutex.h \
utils/xml.cpp \
utils/xml.h \
animatedsprite.cpp \
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. */
};
diff --git a/src/utils/mutex.h b/src/utils/mutex.h
new file mode 100644
index 00000000..62c6b4e1
--- /dev/null
+++ b/src/utils/mutex.h
@@ -0,0 +1,97 @@
+/*
+ * The Mana World
+ * Copyright 2008 The Mana World Development Team
+ *
+ * This file is part of The Mana World.
+ *
+ * The Mana World is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * any later version.
+ *
+ * The Mana World is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with The Mana World; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef TMW_MUTEX_H
+#define TMW_MUTEX_H
+
+#include <SDL_thread.h>
+
+#include "../log.h"
+
+/**
+ * A mutex provides mutual exclusion of access to certain data that is
+ * accessed by multiple threads.
+ */
+class Mutex
+{
+public:
+ Mutex();
+ ~Mutex();
+
+ void lock();
+ void unlock();
+
+private:
+ Mutex(const Mutex&); // prevent copying
+ Mutex& operator=(const Mutex&);
+
+ SDL_mutex *mMutex;
+};
+
+/**
+ * A convenience class for locking a mutex.
+ */
+class MutexLocker
+{
+public:
+ MutexLocker(Mutex *mutex);
+ ~MutexLocker();
+
+private:
+ Mutex *mMutex;
+};
+
+
+inline Mutex::Mutex()
+{
+ mMutex = SDL_CreateMutex();
+}
+
+inline Mutex::~Mutex()
+{
+ SDL_DestroyMutex(mMutex);
+}
+
+inline void Mutex::lock()
+{
+ if (SDL_mutexP(mMutex) == -1)
+ logger->log("Mutex locking failed: %s", SDL_GetError());
+}
+
+inline void Mutex::unlock()
+{
+ if (SDL_mutexV(mMutex) == -1)
+ logger->log("Mutex unlocking failed: %s", SDL_GetError());
+}
+
+
+inline MutexLocker::MutexLocker(Mutex *mutex):
+ mMutex(mutex)
+{
+ mMutex->lock();
+}
+
+inline MutexLocker::~MutexLocker()
+{
+ mMutex->unlock();
+}
+
+#endif // TMW_MUTEX_H