Skip to content

Commit

Permalink
libcore|TaskPool: Redesigned shutdown procedure
Browse files Browse the repository at this point in the history
Previously TaskPool's destructor waited until all the tasks were
finished. Now, the public TaskPool is deleted immediately and the
private instance will wait for the tasks to report themselves finished.
Only after this the private instance deletes itself.

Note: When encountering a crash involving mutex unlocking, check that
a Guarded object isn't deleted during the guarding.

Also cleaned up the friend relationship between Task/TaskPool.
  • Loading branch information
skyjake committed Oct 21, 2014
1 parent d426cee commit 8c9d2fa
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 28 deletions.
6 changes: 2 additions & 4 deletions doomsday/libcore/include/de/concurrency/task.h
Expand Up @@ -22,11 +22,10 @@
#include <QRunnable>

#include "../libcore.h"
#include "../TaskPool"

namespace de {

class TaskPool;

/**
* Concurrent task that will be executed asynchronously by a TaskPool. Override
* runTask() in a derived class.
Expand All @@ -37,7 +36,6 @@ class DENG2_PUBLIC Task : public QRunnable
Task();
virtual ~Task() {}

TaskPool &pool() const;
void run();

/**
Expand All @@ -48,7 +46,7 @@ class DENG2_PUBLIC Task : public QRunnable
private:
friend class TaskPool;

TaskPool *_pool;
TaskPool::IPool *_pool;
};

} // namespace de
Expand Down
14 changes: 11 additions & 3 deletions doomsday/libcore/include/de/concurrency/taskpool.h
Expand Up @@ -55,9 +55,20 @@ class DENG2_PUBLIC TaskPool : public QObject
HighPriority = 2
};

class IPool
{
public:
virtual void taskFinishedRunning(Task &) = 0;
};

public:
TaskPool();

/**
* Destroys the task pool when all running tasks have finished. This method will
* always return immediately and the public-facing TaskPool object will be deleted,
* but the private instance will exist until all the tasks have finished running.
*/
virtual ~TaskPool();

/**
Expand All @@ -84,9 +95,6 @@ class DENG2_PUBLIC TaskPool : public QObject
void allTasksDone();

private:
friend class Task;
void taskFinished(Task &task);

DENG2_PRIVATE(d)
};

Expand Down
10 changes: 2 additions & 8 deletions doomsday/libcore/src/concurrency/task.cpp
Expand Up @@ -22,15 +22,9 @@

namespace de {

Task::Task() : _pool(0)
Task::Task() : _pool(nullptr)
{}

TaskPool &Task::pool() const
{
DENG2_ASSERT(_pool != 0);
return *_pool;
}

void Task::run()
{
try
Expand All @@ -44,7 +38,7 @@ void Task::run()
}

// Cleanup.
if(_pool) _pool->taskFinished(*this);
if(_pool) _pool->taskFinishedRunning(*this);
Log::disposeThreadLog();
}

Expand Down
55 changes: 42 additions & 13 deletions doomsday/libcore/src/concurrency/taskpool.cpp
Expand Up @@ -27,8 +27,10 @@

namespace de {

DENG2_PIMPL(TaskPool), public Lockable, public Waitable
DENG2_PIMPL(TaskPool), public Lockable, public Waitable, public TaskPool::IPool
{
bool deleteWhenDone { false }; ///< Private instance will be deleted when pool is empty.

// Set of running tasks.
typedef QSet<Task *> Tasks;
Tasks tasks;
Expand All @@ -41,13 +43,15 @@ DENG2_PIMPL(TaskPool), public Lockable, public Waitable

~Instance()
{
waitForEmpty();
// The pool is always empty at this point because the destructor is not
// called until all the tasks have been finished and removed.
DENG2_ASSERT(tasks.isEmpty());
}

void add(Task *t)
{
DENG2_GUARD(this);
t->_pool = &self;
t->_pool = this;
if(tasks.isEmpty())
{
wait(); // Semaphore now unavailable.
Expand Down Expand Up @@ -81,13 +85,47 @@ DENG2_PIMPL(TaskPool), public Lockable, public Waitable
DENG2_GUARD(this);
return tasks.isEmpty();
}

void taskFinishedRunning(Task &task)
{
lock();
if(remove(&task))
{
if(deleteWhenDone)
{
// All done, clean up!
unlock();

// NOTE: Guard isn't used because the object doesn't exist past this point.
delete this;
return;
}
else
{
unlock();
emit self.allTasksDone();
}
}
else
{
unlock();
}
}
};

TaskPool::TaskPool() : d(new Instance(this))
{}

TaskPool::~TaskPool()
{}
{
DENG2_GUARD(d);
if(!d->isEmpty())
{
// Detach the private instance and make it auto-delete itself when done.
// The ongoing tasks will report themselves finished to the private instance.
d.release()->deleteWhenDone = true;
}
}

void TaskPool::start(Task *task, Priority priority)
{
Expand All @@ -105,14 +143,5 @@ bool TaskPool::isDone() const
return d->isEmpty();
}

void TaskPool::taskFinished(Task &task)
{
DENG2_GUARD(d);
if(d->remove(&task))
{
emit allTasksDone();
}
}

} // namespace de

0 comments on commit 8c9d2fa

Please sign in to comment.