shill: Run termination actions before stopping DBus proxies.
shill uses a termination action to disable the cellular device upon
termination (CL:171374). It needs to execute the termination action
before calling Daemon::Stop(), which stops the DBus proxies to
ModemManager. Otherwise, the termination action will fail to initiate
the disable operation over ModemManager's DBus interface.
shill previously disconnected but didn't disable the cellular device
upon termination, and thus called Daemon::Stop() before running the
termination action in order to prevent the autoconnect code from trying
to reconnect the disconnected cellular service. However, that's not
necessary as Manager::AutoConnect() actually checks if the manager is
still running.
BUG=chromium:305445
TEST=Manually test the following with E396, E362, ALT3100, MU736 modem:
1. Enable shill verbose logging under a root shell:
ff_debug manager+device+service+cellular+modem
ff_debug --level -5
2. Connect to Ethernet, WiFi, and cellular.
3. Run `stop shill` under a root shell to terminate the shill process.
4. Verify from /var/log/net.log that shill starts the termination
actions and completes them before destroying DBus proxies to
ModemManager. WiFi is disconnected and cellular is disabled.
shill: [1008/170201:INFO:shill_main.cc(122)] Shutting down due to received signal.
shill: [1008/170201:INFO:manager.cc(1113)] Running termination actions.
shill: [1008/170201:INFO:cellular.cc(802)] StartTermination
...
shill: [1008/170204:INFO:cellular.cc(819)] OnTerminationCompleted: org.chromium.flimflam.Error.Success:Success (no error)
...
shill: [1008/170211:INFO:modem_manager.cc(50)] Stop watching modem manager service: org.chromium.ModemManager
shill: [1008/170211:INFO:modem_manager.cc(50)] Stop watching modem manager service: org.freedesktop.ModemManager1
...
shill: [1008/170211:VERBOSE2:shill_main.cc(115)] DeleteDBusControl
Change-Id: I424f79036f07b7aacf795389dd06654c7ece4606
Reviewed-on: https://chromium-review.googlesource.com/172330
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Thieu Le <thieule@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>
diff --git a/shill_daemon.cc b/shill_daemon.cc
index e0a8158..2558fe6 100644
--- a/shill_daemon.cc
+++ b/shill_daemon.cc
@@ -71,13 +71,11 @@
void Daemon::Quit() {
SLOG(Daemon, 1) << "Starting termination actions.";
- // Stop() prevents autoconnect from attempting to immediately connect to
- // services after they have been disconnected.
- Stop();
if (!manager_->RunTerminationActionsAndNotifyMetrics(
Bind(&Daemon::TerminationActionsCompleted, Unretained(this)),
Metrics::kTerminationActionReasonTerminate)) {
SLOG(Daemon, 1) << "No termination actions were run";
+ Stop();
dispatcher_.PostTask(MessageLoop::QuitClosure());
}
}
@@ -86,6 +84,7 @@
SLOG(Daemon, 1) << "Finished termination actions. Result: " << error;
metrics_->NotifyTerminationActionsCompleted(
Metrics::kTerminationActionReasonTerminate, error.IsSuccess());
+ Stop();
dispatcher_.PostTask(MessageLoop::QuitClosure());
}