shill: Fix a remove-while-iterating issue in HookTable.
HookTable::Run() previously iterated through the list of actions and
invoked the start callback of each action, which was not safe because an
action that completes inline could call HookTable::Remove() to remove
itself from the list. This CL fixes the issue by iterating through a
list of start callback of each action.
BUG=chromium:380526
TEST=Tested the following:
1. FEATURES=test emerge-$BOARD platform2`
2. Verify that shill executes the termination action upon suspend on a
DUT with a cellular device.
Change-Id: Ie32a567d8de1017ff1262ba5e625e5a7a00cf01f
Reviewed-on: https://chromium-review.googlesource.com/202538
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>
diff --git a/hook_table.cc b/hook_table.cc
index 418ce07..8d5f3ab 100644
--- a/hook_table.cc
+++ b/hook_table.cc
@@ -4,6 +4,7 @@
#include "shill/hook_table.h"
+#include <list>
#include <string>
#include <base/bind.h>
@@ -19,6 +20,7 @@
using base::Closure;
using base::ConstRef;
using base::Unretained;
+using std::list;
using std::string;
namespace shill {
@@ -26,7 +28,7 @@
HookTable::HookTable(EventDispatcher *event_dispatcher)
: event_dispatcher_(event_dispatcher) {}
-void HookTable::Add(const string &name, const base::Closure &start) {
+void HookTable::Add(const string &name, const Closure &start) {
Remove(name);
hook_table_.insert(
HookTableMap::value_type(name, HookAction(start)));
@@ -74,14 +76,21 @@
// Otherwise, if the first action completes inline, its call to
// ActionComplete() will cause the |done| callback to be invoked before the
// rest of the actions get started.
+ //
+ // An action that completes inline could call HookTable::Remove(), which
+ // modifies |hook_table_|. It is thus not safe to iterate through
+ // |hook_table_| to execute the actions. Instead, we keep a list of start
+ // callback of each action and iterate through that to invoke the callback.
+ list<Closure> action_start_callbacks;
for (auto &hook_entry : hook_table_) {
HookAction *action = &hook_entry.second;
+ action_start_callbacks.push_back(action->start);
action->started = true;
action->completed = false;
}
// Now start the actions.
- for (auto &hook_entry : hook_table_) {
- hook_entry.second.start.Run();
+ for (auto &callback : action_start_callbacks) {
+ callback.Run();
}
}