diff --git a/src/tasks/AbandonableTask.cpp b/src/tasks/AbandonableTask.cpp index 7211a2e05..2d91703b4 100644 --- a/src/tasks/AbandonableTask.cpp +++ b/src/tasks/AbandonableTask.cpp @@ -2,79 +2,18 @@ #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() +void AbandonableTask::doWork_wrapper() { - thDone = false; - done = false; - thAbandoned = false; - progress = 0; - status = ""; - before(); - pthread_mutex_init (&taskMutex, NULL); - pthread_create(&doWorkThread, 0, &AbandonableTask::doWork_helper, this); + Task::doWork_wrapper(); + pthread_cond_signal(&done_cv); -#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); - pthread_cond_signal(&task->done_cv); - bool abandoned = task->thAbandoned; - pthread_mutex_unlock(&task->taskMutex); + pthread_mutex_lock(&taskMutex); + bool abandoned = thAbandoned; + pthread_mutex_unlock(&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; + delete this; } - return NULL; } void AbandonableTask::Finish() @@ -87,13 +26,11 @@ void AbandonableTask::Finish() pthread_mutex_unlock(&taskMutex); // Poll to make sure that the rest of the Task knows that it's - // done, not just us. + // done, not just us. This has to be done because the thread that started + // the AbandonableTask may or may not call Poll before calling Finish. + // This may call callbacks. Poll(); -#ifdef DEBUGTHREADS - std::cerr << "AbandonableTask @ " << this << " finished" << std::endl; -#endif - delete this; } @@ -106,65 +43,32 @@ void AbandonableTask::Abandon() // 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). + // AbandonableTask after unlocking the mutex. 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. + // AbandonableTask::doWork_wrapper 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() +AbandonableTask::AbandonableTask() : + thAbandoned(false) { pthread_cond_init(&done_cv, NULL); -#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 pthread_cond_destroy(&done_cv); } diff --git a/src/tasks/AbandonableTask.h b/src/tasks/AbandonableTask.h index 14920370e..372410c55 100644 --- a/src/tasks/AbandonableTask.h +++ b/src/tasks/AbandonableTask.h @@ -8,16 +8,14 @@ class AbandonableTask : public Task pthread_cond_t done_cv; public: - void Start() override; void Finish(); void Abandon(); - void Poll() override; AbandonableTask(); virtual ~AbandonableTask(); protected: + void doWork_wrapper() override; bool thAbandoned; - TH_ENTRY_POINT static void * doWork_helper(void * ref); }; #endif /* ABANDONABLETASK_H_ */ diff --git a/src/tasks/Task.cpp b/src/tasks/Task.cpp index 761143e9b..85d2a72fe 100644 --- a/src/tasks/Task.cpp +++ b/src/tasks/Task.cpp @@ -11,14 +11,11 @@ void Task::AddTaskListener(TaskListener * listener) void Task::Start() { - thDone = false; - done = false; - progress = 0; - status = ""; - //taskMutex = PTHREAD_MUTEX_INITIALIZER; before(); - pthread_mutex_init (&taskMutex, NULL); + // This would use a lambda if we didn't use pthreads and if I dared omit + // the TH_ENTRY_POINT from the function type. pthread_create(&doWorkThread, 0, &Task::doWork_helper, this); + pthread_detach(doWorkThread); } int Task::GetProgress() @@ -83,24 +80,25 @@ void Task::Poll() if(newDone!=done) { done = newDone; - - pthread_join(doWorkThread, NULL); - pthread_mutex_destroy(&taskMutex); - after(); - notifyDoneMain(); } } } +Task::Task() : + progress(0), + done(false), + thProgress(0), + thDone(false), + listener(NULL) +{ + pthread_mutex_init(&taskMutex, NULL); +} + Task::~Task() { - if(!done) - { - pthread_join(doWorkThread, NULL); - pthread_mutex_destroy(&taskMutex); - } + pthread_mutex_destroy(&taskMutex); } void Task::before() @@ -123,13 +121,18 @@ void Task::after() } -TH_ENTRY_POINT void * Task::doWork_helper(void * ref) +void Task::doWork_wrapper() { - bool newSuccess = ((Task*)ref)->doWork(); - pthread_mutex_lock(&((Task*)ref)->taskMutex); - ((Task*)ref)->thSuccess = newSuccess; - ((Task*)ref)->thDone = true; - pthread_mutex_unlock(&((Task*)ref)->taskMutex); + bool newSuccess = doWork(); + pthread_mutex_lock(&taskMutex); + thSuccess = newSuccess; + thDone = true; + pthread_mutex_unlock(&taskMutex); +} + +TH_ENTRY_POINT void *Task::doWork_helper(void *ref) +{ + ((Task *)ref)->doWork_wrapper(); return NULL; } diff --git a/src/tasks/Task.h b/src/tasks/Task.h index 32ba631cc..4858974c3 100644 --- a/src/tasks/Task.h +++ b/src/tasks/Task.h @@ -17,7 +17,7 @@ public: String GetError(); String GetStatus(); virtual void Poll(); - Task() : listener(NULL) { progress = 0; thProgress = 0; } + Task(); virtual ~Task(); protected: int progress; @@ -35,12 +35,12 @@ protected: TaskListener * listener; pthread_t doWorkThread; pthread_mutex_t taskMutex; - pthread_cond_t taskCond; virtual void before(); virtual void after(); virtual bool doWork(); + virtual void doWork_wrapper(); TH_ENTRY_POINT static void * doWork_helper(void * ref); virtual void notifyProgress(int progress);