diff --git a/src/mbgl/actor/scheduler.cpp b/src/mbgl/actor/scheduler.cpp index 707de734d..68a55cbef 100644 --- a/src/mbgl/actor/scheduler.cpp +++ b/src/mbgl/actor/scheduler.cpp @@ -48,29 +48,22 @@ std::shared_ptr Scheduler::GetBackground() { // static std::shared_ptr Scheduler::GetSequenced() { - const std::size_t kSchedulersCount = 10; + constexpr std::size_t kSchedulersCount = 10; static std::vector> weaks(kSchedulersCount); static std::mutex mtx; static std::size_t lastUsedIndex = 0u; - std::lock_guard lock(mtx); + std::lock_guard lock(mtx); - if (++lastUsedIndex == kSchedulersCount) lastUsedIndex = 0u; + lastUsedIndex = (lastUsedIndex + 1) % kSchedulersCount; - std::shared_ptr result; - for (std::size_t i = 0; i < kSchedulersCount; ++i) { - auto& weak = weaks[i]; - if (auto scheduler = weak.lock()) { - if (lastUsedIndex == i) result = scheduler; - continue; - } - result = std::make_shared(); - weak = result; - lastUsedIndex = i; - break; + if (auto scheduler = weaks[lastUsedIndex].lock()) { + return scheduler; + } else { + auto result = std::make_shared(); + weaks[lastUsedIndex] = result; + return result; } - - return result; } } // namespace mbgl diff --git a/test/util/async_task.test.cpp b/test/util/async_task.test.cpp index fd58cfd5a..1fc7acf53 100644 --- a/test/util/async_task.test.cpp +++ b/test/util/async_task.test.cpp @@ -206,13 +206,22 @@ TEST(AsyncTask, SequencedScheduler) { } TEST(AsyncTask, MultipleSequencedSchedulers) { - std::vector> shedulers; + constexpr std::size_t kSchedulersCount = 10; // must match the value in the scheduler - for (int i = 0; i < 10; ++i) { - std::shared_ptr scheduler = Scheduler::GetSequenced(); - EXPECT_TRUE(std::none_of( - shedulers.begin(), shedulers.end(), [&scheduler](const auto &item) { return item == scheduler; })); - shedulers.emplace_back(std::move(scheduler)); + std::vector> schedulers; + + // Regression check, the scheduler assignment was previously sensitive to the state of the weak references. + // If expired weak references followed a still-valid one, both after the last-used index, the index would + // be incremented multiple times. + auto temp = Scheduler::GetSequenced(); + temp = Scheduler::GetSequenced(); + + // Check that exactly N unique schedulers are produced. + // Note that this relies on no other threads requesting schedulers. + for (std::size_t i = 0; i < kSchedulersCount; ++i) { + auto scheduler = Scheduler::GetSequenced(); + EXPECT_TRUE(std::ranges::find(schedulers, scheduler) == schedulers.end()); + schedulers.emplace_back(std::move(scheduler)); } - EXPECT_EQ(shedulers.front(), std::shared_ptr(Scheduler::GetSequenced())); + EXPECT_EQ(schedulers.front(), Scheduler::GetSequenced()); }