A Bug in the Priority Scheduler for Coroutines
In my last two posts, I presented priority schedulers for coroutines. The schedulers had a bug.
First of all, here is the erroneous scheduler.
// priority_queueSchedulerPriority.cpp #include <concepts> #include <coroutine> #include <functional> #include <iostream> #include <queue> #include <utility> struct Task { struct promise_type { std::suspend_always initial_suspend() noexcept { return {}; } std::suspend_always final_suspend() noexcept { return {}; } Task get_return_object() { return std::coroutine_handle<promise_type>::from_promise(*this); } void return_void() {} void unhandled_exception() {} }; Task(std::coroutine_handle<promise_type> handle): handle{handle}{} auto get_handle() { return handle; } std::coroutine_handle<promise_type> handle; }; using job = std::pair<int, std::coroutine_handle<>>; template <typename Updater = std::identity, // (1) typename Comperator = std::ranges::less> requires std::invocable<decltype(Updater()), int> && // (2) std::predicate<decltype(Comperator()), job, job> class Scheduler { std::priority_queue<job, std::vector<job>, Comperator> _prioTasks; public: void emplace(int prio, std::coroutine_handle<> task) { _prioTasks.push(std::make_pair(prio, task)); } void schedule() { Updater upd = {}; // (3) while(!_prioTasks.empty()) { auto [prio, task] = _prioTasks.top(); _prioTasks.pop(); task.resume(); if(!task.done()) { _prioTasks.push(std::make_pair(upd(prio), task)); // (4) } else { task.destroy(); } } } }; Task createTask(const std::string& name) { std::cout << name << " start\n"; co_await std::suspend_always(); for (int i = 0; i <= 3; ++i ) { std::cout << name << " execute " << i << "\n"; // (5) co_await std::suspend_always(); } co_await std::suspend_always(); std::cout << name << " finish\n"; } int main() { std::cout << '\n'; Scheduler scheduler1; // (6) scheduler1.emplace(0, createTask("TaskA").get_handle()); scheduler1.emplace(1, createTask(" TaskB").get_handle()); scheduler1.emplace(2, createTask(" TaskC").get_handle()); scheduler1.schedule(); std::cout << '\n'; Scheduler<decltype([](int a) { return a - 1; })> scheduler2; // (7) scheduler2.emplace(0, createTask("TaskA").get_handle()); scheduler2.emplace(1, createTask(" TaskB").get_handle()); scheduler2.emplace(2, createTask(" TaskC").get_handle()); scheduler2.schedule(); std::cout << '\n'; }
This is the output of the program I got.
Christof Meerwald got another output with GCC. Thanks a lot for letting me know. Here is the GCC output with optimization enabled.
Similarly, the Windows output was also broken:
Can you spot the error? Here are the crucial lines:
Task createTask(const std::string& name) { // (1) std::cout << name << " start\n"; co_await std::suspend_always(); for (int i = 0; i <= 3; ++i ) { std::cout << name << " execute " << i << "\n"; co_await std::suspend_always(); } co_await std::suspend_always(); std::cout << name << " finish\n"; } int main() { std::cout << '\n'; Scheduler scheduler1; scheduler1.emplace(0, createTask("TaskA").get_handle()); // (2) scheduler1.emplace(1, createTask(" TaskB").get_handle()); // (3) scheduler1.emplace(2, createTask(" TaskC").get_handle()); // (4) scheduler1.schedule(); std::cout << '\n'; Scheduler<decltype([](int a) { return a - 1; })> scheduler2; scheduler2.emplace(0, createTask("TaskA").get_handle()); // (5) scheduler2.emplace(1, createTask(" TaskB").get_handle()); // (6) scheduler2.emplace(2, createTask(" TaskC").get_handle()); // (7) scheduler2.schedule(); std::cout << '\n'; }
The coroutine createTask
takes its string by const lvalue reference (1), but its arguments "TaskA" - "TaskC"
are rvalues (lines 2 – 7). Using the reference to the temporaries is undefined behavior. The additional schedulers priority_SchedulerSimplified
and priority_queueSchedulerComparator
in the posts “A Priority Scheduler for Coroutines” and “An Advanced Priority Scheduler for Coroutines” suffer the same issue.
Fixing the issue is straightforward. Either the coroutine createTask
takes its argument by value (Task createTask(std::string name
)) or its arguments become lvalues. Here is the second approach in lines (1) – (3):
// priority_queueSchedulerPriority.cpp #include <concepts> #include <coroutine> #include <functional> #include <iostream> #include <queue> #include <utility> struct Task { struct promise_type { std::suspend_always initial_suspend() noexcept { return {}; } std::suspend_always final_suspend() noexcept { return {}; } Task get_return_object() { return std::coroutine_handle<promise_type>::from_promise(*this); } void return_void() {} void unhandled_exception() {} }; Task(std::coroutine_handle<promise_type> handle): handle{handle}{} auto get_handle() { return handle; } std::coroutine_handle<promise_type> handle; }; using job = std::pair<int, std::coroutine_handle<>>; template <typename Updater = std::identity, typename Comperator = std::ranges::less> requires std::invocable<decltype(Updater()), int> && std::predicate<decltype(Comperator()), job, job> class Scheduler { std::priority_queue<job, std::vector<job>, Comperator> _prioTasks; public: void emplace(int prio, std::coroutine_handle<> task) { _prioTasks.push(std::make_pair(prio, task)); } void schedule() { Updater upd = {}; while(!_prioTasks.empty()) { auto [prio, task] = _prioTasks.top(); _prioTasks.pop(); task.resume(); if(!task.done()) { _prioTasks.push(std::make_pair(upd(prio), task)); } else { task.destroy(); } } } }; Task createTask(const std::string& name) { std::cout << name << " start\n"; co_await std::suspend_always(); for (int i = 0; i <= 3; ++i ) { std::cout << name << " execute " << i << "\n"; co_await std::suspend_always(); } co_await std::suspend_always(); std::cout << name << " finish\n"; } int main() { std::cout << '\n'; std::string taskA = "TaskA"; // (1) std::string taskB = " TaskB"; // (2) std::string taskC = " TaskC"; // (3) Scheduler scheduler1; scheduler1.emplace(0, createTask(taskA).get_handle()); scheduler1.emplace(1, createTask(taskB).get_handle()); scheduler1.emplace(2, createTask(taskC).get_handle()); scheduler1.schedule(); std::cout << '\n'; Scheduler<decltype([](int a) { return a - 1; })> scheduler2; scheduler2.emplace(0, createTask(taskA).get_handle()); scheduler2.emplace(1, createTask(taskB).get_handle()); scheduler2.emplace(2, createTask(taskC).get_handle()); scheduler2.schedule(); std::cout << '\n'; }
What’s Next?
Coroutines provide an intuitive way of writing asynchronous code. My next post will be a guest post from Ljubic Damir, presenting a single producer – single consumer workflow based on coroutines.
Thanks a lot to my Patreon Supporters: Matt Braun, Roman Postanciuc, Tobias Zindl, G Prvulovic, Reinhold Dröge, Abernitzke, Frank Grimm, Sakib, Broeserl, António Pina, Sergey Agafyin, Андрей Бурмистров, Jake, GS, Lawton Shoemake, Jozo Leko, John Breland, Venkat Nandam, Jose Francisco, Douglas Tinkham, Kuchlong Kuchlong, Robert Blanch, Truels Wissneth, Mario Luoni, Friedrich Huber, lennonli, Pramod Tikare Muralidhara, Peter Ware, Daniel Hufschläger, Alessandro Pezzato, Bob Perry, Satish Vangipuram, Andi Ireland, Richard Ohnemus, Michael Dunsky, Leo Goodstadt, John Wiederhirn, Yacob Cohen-Arazi, Florian Tischler, Robin Furness, Michael Young, Holger Detering, Bernd Mühlhaus, Stephen Kelley, Kyle Dean, Tusar Palauri, Juan Dent, George Liao, Daniel Ceperley, Jon T Hess, Stephen Totten, Wolfgang Fütterer, Matthias Grün, Phillip Diekmann, Ben Atakora, Ann Shatoff, Rob North, Bhavith C Achar, Marco Parri Empoli, Philipp Lenk, Charles-Jianye Chen, Keith Jeffery,and Matt Godbolt.
Modernes C++ Mentoring
Do you want to stay informed: Subscribe.
Thanks, in particular, to Jon Hess, Lakshman, Christian Wittenhorst, Sherhy Pyton, Dendi Suhubdy, Sudhakar Belagurusamy, Richard Sargeant, Rusty Fleming, John Nebel, Mipko, Alicja Kaminska, Slavko Radman, and David Poole.
My special thanks to Embarcadero | |
My special thanks to PVS-Studio | |
My special thanks to Tipi.build | |
My special thanks to Take Up Code | |
My special thanks to SHAVEDYAKS |
Modernes C++ GmbH
Modernes C++ Mentoring (English)
Rainer Grimm
Yalovastraße 20
72108 Rottenburg
Mail: schulung@ModernesCpp.de
Mentoring: www.ModernesCpp.org
Modernes C++ Mentoring,