[scheduler] Fix use-after-move crash in heap operations (#12124)
This commit is contained in:
committed by
Jonathan Swoboda
parent
b4b34aee13
commit
d5e2543751
@@ -359,8 +359,7 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
std::unique_ptr<SchedulerItem> item;
|
||||
{
|
||||
LockGuard guard{this->lock_};
|
||||
item = std::move(this->items_[0]);
|
||||
this->pop_raw_();
|
||||
item = this->pop_raw_locked_();
|
||||
}
|
||||
|
||||
const char *name = item->get_name();
|
||||
@@ -401,7 +400,7 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
// Don't run on failed components
|
||||
if (item->component != nullptr && item->component->is_failed()) {
|
||||
LockGuard guard{this->lock_};
|
||||
this->pop_raw_();
|
||||
this->recycle_item_(this->pop_raw_locked_());
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -414,7 +413,7 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
{
|
||||
LockGuard guard{this->lock_};
|
||||
if (is_item_removed_(item.get())) {
|
||||
this->pop_raw_();
|
||||
this->recycle_item_(this->pop_raw_locked_());
|
||||
this->to_remove_--;
|
||||
continue;
|
||||
}
|
||||
@@ -423,7 +422,7 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
// Single-threaded or multi-threaded with atomics: can check without lock
|
||||
if (is_item_removed_(item.get())) {
|
||||
LockGuard guard{this->lock_};
|
||||
this->pop_raw_();
|
||||
this->recycle_item_(this->pop_raw_locked_());
|
||||
this->to_remove_--;
|
||||
continue;
|
||||
}
|
||||
@@ -443,14 +442,14 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
|
||||
LockGuard guard{this->lock_};
|
||||
|
||||
auto executed_item = std::move(this->items_[0]);
|
||||
// Only pop after function call, this ensures we were reachable
|
||||
// during the function call and know if we were cancelled.
|
||||
this->pop_raw_();
|
||||
auto executed_item = this->pop_raw_locked_();
|
||||
|
||||
if (executed_item->remove) {
|
||||
// We were removed/cancelled in the function call, stop
|
||||
// We were removed/cancelled in the function call, recycle and continue
|
||||
this->to_remove_--;
|
||||
this->recycle_item_(std::move(executed_item));
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -497,7 +496,7 @@ size_t HOT Scheduler::cleanup_() {
|
||||
return this->items_.size();
|
||||
|
||||
// We must hold the lock for the entire cleanup operation because:
|
||||
// 1. We're modifying items_ (via pop_raw_) which requires exclusive access
|
||||
// 1. We're modifying items_ (via pop_raw_locked_) which requires exclusive access
|
||||
// 2. We're decrementing to_remove_ which is also modified by other threads
|
||||
// (though all modifications are already under lock)
|
||||
// 3. Other threads read items_ when searching for items to cancel in cancel_item_locked_()
|
||||
@@ -510,17 +509,18 @@ size_t HOT Scheduler::cleanup_() {
|
||||
if (!item->remove)
|
||||
break;
|
||||
this->to_remove_--;
|
||||
this->pop_raw_();
|
||||
this->recycle_item_(this->pop_raw_locked_());
|
||||
}
|
||||
return this->items_.size();
|
||||
}
|
||||
void HOT Scheduler::pop_raw_() {
|
||||
std::unique_ptr<Scheduler::SchedulerItem> HOT Scheduler::pop_raw_locked_() {
|
||||
std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp);
|
||||
|
||||
// Instead of destroying, recycle the item
|
||||
this->recycle_item_(std::move(this->items_.back()));
|
||||
// Move the item out before popping - this is the item that was at the front of the heap
|
||||
auto item = std::move(this->items_.back());
|
||||
|
||||
this->items_.pop_back();
|
||||
return item;
|
||||
}
|
||||
|
||||
// Helper to execute a scheduler item
|
||||
|
||||
@@ -219,7 +219,9 @@ class Scheduler {
|
||||
// Returns the number of items remaining after cleanup
|
||||
// IMPORTANT: This method should only be called from the main thread (loop task).
|
||||
size_t cleanup_();
|
||||
void pop_raw_();
|
||||
// Remove and return the front item from the heap
|
||||
// IMPORTANT: Caller must hold the scheduler lock before calling this function.
|
||||
std::unique_ptr<SchedulerItem> pop_raw_locked_();
|
||||
|
||||
private:
|
||||
// Helper to cancel items by name - must be called with lock held
|
||||
|
||||
Reference in New Issue
Block a user