diff --git a/src/client/ThumbnailRenderer.cpp b/src/client/ThumbnailRendererTask.cpp similarity index 86% rename from src/client/ThumbnailRenderer.cpp rename to src/client/ThumbnailRendererTask.cpp index 126497e78..f44332863 100644 --- a/src/client/ThumbnailRenderer.cpp +++ b/src/client/ThumbnailRendererTask.cpp @@ -1,4 +1,4 @@ -#include "ThumbnailRenderer.h" +#include "ThumbnailRendererTask.h" #include @@ -47,8 +47,10 @@ bool ThumbnailRendererTask::doWork() } } -std::unique_ptr ThumbnailRendererTask::GetThumbnail() +std::unique_ptr ThumbnailRendererTask::Finish() { - return std::move(thumbnail); + auto ptr = std::move(thumbnail); + AbandonableTask::Finish(); + return ptr; } diff --git a/src/client/ThumbnailRenderer.h b/src/client/ThumbnailRendererTask.h similarity index 79% rename from src/client/ThumbnailRenderer.h rename to src/client/ThumbnailRendererTask.h index 8a8b5ae2d..21c0a845a 100644 --- a/src/client/ThumbnailRenderer.h +++ b/src/client/ThumbnailRendererTask.h @@ -1,13 +1,13 @@ #ifndef THUMBNAILRENDERER_H #define THUMBNAILRENDERER_H -#include "tasks/Task.h" +#include "tasks/AbandonableTask.h" #include class GameSave; class VideoBuffer; -class ThumbnailRendererTask : public Task +class ThumbnailRendererTask : public AbandonableTask { std::unique_ptr Save; int Width, Height; @@ -21,7 +21,7 @@ public: virtual ~ThumbnailRendererTask(); virtual bool doWork() override; - std::unique_ptr GetThumbnail(); + std::unique_ptr Finish(); }; #endif // THUMBNAILRENDERER_H diff --git a/src/gui/interface/SaveButton.cpp b/src/gui/interface/SaveButton.cpp index 419c8d57f..8d589e500 100644 --- a/src/gui/interface/SaveButton.cpp +++ b/src/gui/interface/SaveButton.cpp @@ -8,7 +8,7 @@ #include "SaveButton.h" #include "client/Client.h" #include "client/SaveInfo.h" -#include "client/ThumbnailRenderer.h" +#include "client/ThumbnailRendererTask.h" #include "simulation/SaveRenderer.h" #include "client/GameSave.h" #include "simulation/SaveRenderer.h" @@ -23,6 +23,7 @@ SaveButton::SaveButton(Point position, Point size, SaveInfo * save): isMouseInsideAuthor(false), isMouseInsideHistory(false), showVotes(false), + thumbnailRenderer(nullptr), isButtonDown(false), isMouseInside(false), selected(false), @@ -96,6 +97,7 @@ SaveButton::SaveButton(Point position, Point size, SaveFile * file): isMouseInsideAuthor(false), isMouseInsideHistory(false), showVotes(false), + thumbnailRenderer(nullptr), isButtonDown(false), isMouseInside(false), selected(false), @@ -116,6 +118,10 @@ SaveButton::SaveButton(Point position, Point size, SaveFile * file): SaveButton::~SaveButton() { + if (thumbnailRenderer) + { + thumbnailRenderer->Abandon(); + } delete actionCallback; delete save; delete file; @@ -138,7 +144,7 @@ void SaveButton::Tick(float dt) { if(save->GetGameSave()) { - thumbnailRenderer = std::unique_ptr(new ThumbnailRendererTask(save->GetGameSave(), thumbBoxSize.X, thumbBoxSize.Y)); + thumbnailRenderer = new ThumbnailRendererTask(save->GetGameSave(), thumbBoxSize.X, thumbBoxSize.Y); thumbnailRenderer->Start(); triedThumbnail = true; } @@ -151,7 +157,7 @@ void SaveButton::Tick(float dt) } else if (file && file->GetGameSave()) { - thumbnailRenderer = std::unique_ptr(new ThumbnailRendererTask(file->GetGameSave(), thumbBoxSize.X, thumbBoxSize.Y, true, true, false)); + thumbnailRenderer = new ThumbnailRendererTask(file->GetGameSave(), thumbBoxSize.X, thumbBoxSize.Y, true, true, false); thumbnailRenderer->Start(); triedThumbnail = true; } @@ -164,8 +170,8 @@ void SaveButton::Tick(float dt) thumbnailRenderer->Poll(); if (thumbnailRenderer->GetDone()) { - thumbnail = thumbnailRenderer->GetThumbnail(); - thumbnailRenderer.reset(); + thumbnail = thumbnailRenderer->Finish(); + thumbnailRenderer = nullptr; } } diff --git a/src/gui/interface/SaveButton.h b/src/gui/interface/SaveButton.h index bf7aba9b8..84bc9a2cc 100644 --- a/src/gui/interface/SaveButton.h +++ b/src/gui/interface/SaveButton.h @@ -45,7 +45,7 @@ class SaveButton : public Component, public http::RequestMonitor thumbnailRenderer; + ThumbnailRendererTask *thumbnailRenderer; public: SaveButton(Point position, Point size, SaveInfo * save); SaveButton(Point position, Point size, SaveFile * file); diff --git a/src/gui/save/LocalSaveActivity.cpp b/src/gui/save/LocalSaveActivity.cpp index 2256b822a..29bc67b47 100644 --- a/src/gui/save/LocalSaveActivity.cpp +++ b/src/gui/save/LocalSaveActivity.cpp @@ -11,7 +11,7 @@ #include "gui/interface/Button.h" #include "gui/interface/Label.h" #include "gui/interface/Textbox.h" -#include "client/ThumbnailRenderer.h" +#include "client/ThumbnailRendererTask.h" class LocalSaveActivity::CancelAction: public ui::ButtonAction @@ -39,6 +39,7 @@ public: LocalSaveActivity::LocalSaveActivity(SaveFile save, FileSavedCallback * callback) : WindowActivity(ui::Point(-1, -1), ui::Point(220, 200)), save(save), + thumbnailRenderer(nullptr), callback(callback) { ui::Label * titleLabel = new ui::Label(ui::Point(4, 5), ui::Point(Size.X-8, 16), "Save to computer:"); @@ -71,7 +72,7 @@ LocalSaveActivity::LocalSaveActivity(SaveFile save, FileSavedCallback * callback if(save.GetGameSave()) { - thumbnailRenderer = std::unique_ptr(new ThumbnailRendererTask(save.GetGameSave(), Size.X-16, -1, false, true, false)); + thumbnailRenderer = new ThumbnailRendererTask(save.GetGameSave(), Size.X-16, -1, false, true, false); thumbnailRenderer->Start(); } } @@ -83,8 +84,8 @@ void LocalSaveActivity::OnTick(float dt) thumbnailRenderer->Poll(); if (thumbnailRenderer->GetDone()) { - thumbnail = thumbnailRenderer->GetThumbnail(); - thumbnailRenderer.reset(); + thumbnail = thumbnailRenderer->Finish(); + thumbnailRenderer = nullptr; } } } @@ -168,5 +169,9 @@ void LocalSaveActivity::OnDraw() LocalSaveActivity::~LocalSaveActivity() { + if (thumbnailRenderer) + { + thumbnailRenderer->Abandon(); + } delete callback; } diff --git a/src/gui/save/LocalSaveActivity.h b/src/gui/save/LocalSaveActivity.h index 5513a6f5d..02636bcc0 100644 --- a/src/gui/save/LocalSaveActivity.h +++ b/src/gui/save/LocalSaveActivity.h @@ -24,7 +24,7 @@ public: class LocalSaveActivity: public WindowActivity { SaveFile save; - std::unique_ptr thumbnailRenderer; + ThumbnailRendererTask *thumbnailRenderer; std::unique_ptr thumbnail; ui::Textbox * filenameField; class CancelAction; diff --git a/src/gui/save/ServerSaveActivity.cpp b/src/gui/save/ServerSaveActivity.cpp index c2ad10701..e7ba58e5f 100644 --- a/src/gui/save/ServerSaveActivity.cpp +++ b/src/gui/save/ServerSaveActivity.cpp @@ -13,7 +13,7 @@ #include "gui/Style.h" #include "client/GameSave.h" #include "images.h" -#include "client/ThumbnailRenderer.h" +#include "client/ThumbnailRendererTask.h" class ServerSaveActivity::CancelAction: public ui::ButtonAction { @@ -104,6 +104,7 @@ public: ServerSaveActivity::ServerSaveActivity(SaveInfo save, ServerSaveActivity::SaveUploadedCallback * callback) : WindowActivity(ui::Point(-1, -1), ui::Point(440, 200)), + thumbnailRenderer(nullptr), save(save), callback(callback), saveUploadTask(NULL) @@ -184,13 +185,14 @@ ServerSaveActivity::ServerSaveActivity(SaveInfo save, ServerSaveActivity::SaveUp if (save.GetGameSave()) { - thumbnailRenderer = std::unique_ptr(new ThumbnailRendererTask(save.GetGameSave(), (Size.X/2)-16, -1, false, false, true)); + thumbnailRenderer = new ThumbnailRendererTask(save.GetGameSave(), (Size.X/2)-16, -1, false, false, true); thumbnailRenderer->Start(); } } ServerSaveActivity::ServerSaveActivity(SaveInfo save, bool saveNow, ServerSaveActivity::SaveUploadedCallback * callback) : WindowActivity(ui::Point(-1, -1), ui::Point(200, 50)), + thumbnailRenderer(nullptr), save(save), callback(callback), saveUploadTask(NULL) @@ -407,8 +409,8 @@ void ServerSaveActivity::OnTick(float dt) thumbnailRenderer->Poll(); if (thumbnailRenderer->GetDone()) { - thumbnail = thumbnailRenderer->GetThumbnail(); - thumbnailRenderer.reset(); + thumbnail = thumbnailRenderer->Finish(); + thumbnailRenderer = nullptr; } } @@ -435,6 +437,10 @@ void ServerSaveActivity::OnDraw() ServerSaveActivity::~ServerSaveActivity() { + if (thumbnailRenderer) + { + thumbnailRenderer->Abandon(); + } delete saveUploadTask; delete callback; } diff --git a/src/gui/save/ServerSaveActivity.h b/src/gui/save/ServerSaveActivity.h index 86f675e10..34cec5575 100644 --- a/src/gui/save/ServerSaveActivity.h +++ b/src/gui/save/ServerSaveActivity.h @@ -40,7 +40,7 @@ public: protected: void AddAuthorInfo(); virtual void NotifyDone(Task * task); - std::unique_ptr thumbnailRenderer; + ThumbnailRendererTask *thumbnailRenderer; std::unique_ptr thumbnail; SaveInfo save; SaveUploadedCallback * callback; diff --git a/src/tasks/AbandonableTask.cpp b/src/tasks/AbandonableTask.cpp new file mode 100644 index 000000000..dafb14580 --- /dev/null +++ b/src/tasks/AbandonableTask.cpp @@ -0,0 +1,163 @@ +#include "AbandonableTask.h" + +#include "Platform.h" + +#ifdef DEBUG +# define DEBUGTHREADS +// * Uncomment above if AbandonableTasks cause issues. +// * Three outputs should be possible: +// * The task is properly finished: +// AbandonableTask @ [ptr] ctor +// AbandonableTask @ [ptr] created +// ... +// AbandonableTask @ [ptr] finished +// AbandonableTask @ [ptr] joined +// AbandonableTask @ [ptr] dtor +// * The task is abandoned but its thread has finished: +// AbandonableTask @ [ptr] ctor +// AbandonableTask @ [ptr] created +// ... +// AbandonableTask @ [ptr] abandoned +// AbandonableTask @ [ptr] joined +// AbandonableTask @ [ptr] dtor +// * The task is abandoned before its thread has finished: +// AbandonableTask @ [ptr] ctor +// AbandonableTask @ [ptr] created +// ... +// AbandonableTask @ [ptr] abandoned +// ... +// AbandonableTask @ [ptr] detached +// AbandonableTask @ [ptr] dtor +// * Anything other than those means something is broken. +#endif + +#ifdef DEBUGTHREADS +# include +#endif + +void AbandonableTask::Start() +{ + thDone = false; + done = false; + thAbandoned = false; + progress = 0; + status = ""; + //taskMutex = PTHREAD_MUTEX_INITIALIZER; + before(); + pthread_mutex_init (&taskMutex, NULL); + pthread_create(&doWorkThread, 0, &AbandonableTask::doWork_helper, this); + +#ifdef DEBUGTHREADS + std::cerr << "AbandonableTask @ " << this << " created" << std::endl; +#endif +} + +TH_ENTRY_POINT void * AbandonableTask::doWork_helper(void * ref) +{ + Task::doWork_helper(ref); + + AbandonableTask *task = (AbandonableTask *)ref; + pthread_mutex_lock(&task->taskMutex); + bool abandoned = task->thAbandoned; + pthread_mutex_unlock(&task->taskMutex); + if (abandoned) + { + pthread_detach(task->doWorkThread); + pthread_mutex_destroy(&task->taskMutex); + +#ifdef DEBUGTHREADS + std::cerr << "AbandonableTask @ " << ref << " detached" << std::endl; +#endif + + // We've done Task::~Task's job already. + task->done = true; + + delete task; + } + return NULL; +} + +void AbandonableTask::Finish() +{ + while (!GetDone()) + { + Poll(); + Platform::Millisleep(1); + } + +#ifdef DEBUGTHREADS + std::cerr << "AbandonableTask @ " << this << " finished" << std::endl; +#endif + + delete this; +} + +void AbandonableTask::Abandon() +{ + bool delete_this = false; + pthread_mutex_lock(&taskMutex); + if (thDone) + { + // If thDone is true, the thread has already finished. We're + // not calling Poll because it may call callbacks, which + // an abandoned task shouldn't do. Instead we just delete the + // AbandonableTask after unlocking the mutex, which will + // take care of the thread if another Poll hasn't already + // (i.e. if done is false despite thDone being true). + delete_this = true; + } + else + { + // If at this point thDone is still false, the thread is still + // running, meaning we can safely set thAbandoned and let + // AbandonableTask::doWork_helper detach the thread + // and delete the AbandonableTask later. + thAbandoned = true; + } + pthread_mutex_unlock(&taskMutex); + +#ifdef DEBUGTHREADS + std::cerr << "AbandonableTask @ " << this << " abandoned" << std::endl; +#endif + + if (delete_this) + { + delete this; + } +} + +void AbandonableTask::Poll() +{ +#ifdef DEBUGTHREADS + bool old_done = done; +#endif + Task::Poll(); +#ifdef DEBUGTHREADS + if (done != old_done) + { + std::cerr << "AbandonableTask @ " << this << " joined" << std::endl; + } +#endif +} + +AbandonableTask::AbandonableTask() +{ +#ifdef DEBUGTHREADS + std::cerr << "AbandonableTask @ " << this << " ctor" << std::endl; +#endif +} + +AbandonableTask::~AbandonableTask() +{ +#ifdef DEBUGTHREADS + if (!done) + { + // Actually it'll be joined later in Task::~Task, but the debug + // messages look more consistent this way. + std::cerr << "AbandonableTask @ " << this << " joined" << std::endl; + } + + std::cerr << "AbandonableTask @ " << this << " dtor" << std::endl; +#endif +} + diff --git a/src/tasks/AbandonableTask.h b/src/tasks/AbandonableTask.h new file mode 100644 index 000000000..835f50941 --- /dev/null +++ b/src/tasks/AbandonableTask.h @@ -0,0 +1,21 @@ +#ifndef ABANDONABLETASK_H_ +#define ABANDONABLETASK_H_ + +#include "Task.h" + +class AbandonableTask : public Task +{ +public: + void Start(); + void Finish(); + void Abandon(); + void Poll(); + AbandonableTask(); + virtual ~AbandonableTask(); + +protected: + bool thAbandoned; + TH_ENTRY_POINT static void * doWork_helper(void * ref); +}; + +#endif /* ABANDONABLETASK_H_ */