On a busy or slow system, the CScheduler unit test could fail because it assumed all threads would be done after a couple of milliseconds.
Replace the hard-coded sleep with a task that reschedules itself until the task queue is empty, and then shuts down the queue servicing threads.
I thought about this solution, to queue a task after the other tasks that signals the main thread to continue. However, as the handling is multi-threaded it is not guaranteed that the other tasks will have finished execution by the time the closing task runs. I think this will need additional synchronization to be sure.
@laanwj : mmm... I think CScheduler needs a way to do a clean shutdown when there are multiple threads servicing the queue. I'll take a whack at that after lunch.
Alternatively, the unit test could be changed to just use a single thread so task ordering is deterministic; we aren't using multiple threads to service it, anyway.
Added CScheduler::stop(bool drain) method, which is the semantics we really want ("let the threads exit when there is nothing left to do").
Looks good to me.
However, it does not yet address @theuni' comment on #6145
If there's a future bug that would otherwise cause this to fail because the tasks never successfully finish, this would now hang forever rather than failing, no? Maybe give it an absurd timeout, or verify that the numTasksInQueue are decreasing?
If the queue never drains (for example if every task keeps adding new taks), it will keep going on for ever. If the join takes longer than say, 60 seconds this could be flagged as an error as well. Ironically this was easier to accomplish with the loop of #6145.
On the other hand there are plenty of Python unit tests that have 'wait forever' failure scenarios as well, so I don't think this is a big problem. This fixes the immediate issue so utACK.
61 | + while (!stopRequested && !taskQueue.empty() && 62 | + newTaskScheduled.wait_until(lock, taskQueue.begin()->first) != boost::cv_status::timeout) { 63 | // Keep waiting until timeout 64 | } 65 | #endif 66 | + if (stopRequested)
Could move this if (stopRequested) to above the #if BOOST_VERSION < 105000, then the while () conditions don't have to be extended.
On a busy or slow system, the CScheduler unit test could fail because it
assumed all threads would be done after a couple of milliseconds.
Replace the hard-coded sleep with CScheduler stop() method that
will cleanly exit the servicing threads when all tasks are completely
finished.
Nit picked, merged to master (quick merge because Travis errors are so disruptive).