ThreadPool::runNextJob bug

#1

At the top, the part:

[code]bool ThreadPool::runNextJob()
{
lock.enter();
ThreadPoolJob* job = 0;

for (int i = 0; i < jobs.size(); ++i)
{
    job = (ThreadPoolJob*) jobs [i];

    if (job != 0 && ! (job->isActive || job->shouldStop))
        break;
}

if (job != 0)
{
   ...

[/code]

should read:

[code]bool ThreadPool::runNextJob()
{
lock.enter();
ThreadPoolJob* job = 0;

for (int i = 0; i < jobs.size(); ++i)
{
    ThreadPoolJob* jobToRun = (ThreadPoolJob*) jobs [i];

    if (jobToRun != 0 && ! (jobToRun->isActive || jobToRun->shouldStop))
    {
        job = jobToRun;
        break;
    }
}

if (job != 0)
{
   ...

[/code]

otherwise a single job will be run in as many threads the pool can muster…

#2

ooh… yes. Good bug-spotting, thanks!

#3

Thnx Jules, while on the topic, I think a good feature of the thread pool would be that jobs that are finished, optionally should be moved into a destructor queue and be deleted. Otherwise its the responsibility of each user of the pool to delete the jobs they have put into the pool (which I’d say is the normal behaviour and thus should be provided by the pool itself… :slight_smile: )

#4

Actually, that’s not the way I typically use them - I tend to use objects that might get run as a job multiple times, and which do other tasks, so I wouldn’t want them deleted.

But I think you could also do a “removeJob(); delete this” at the end of the runJob() method, which would be pretty easy way to clean up.

#5

Ok, but there’s no removeJob() (in ThreadPoolJob) method I can call, and I can’t get to the pool member as its private. A suggestion would be to add an enum like jobHasFinishedDeleteMe to let the thread pool delete the completed job if it so wishes. Meanwhile, if the pool member is protected, your suggestion should work fine…

#6

Good point. I’ll sort something out with it…

#7