login: change implementation of asynchronous Upstart events
The way Upstart was being triggered could lead to two event emissions
called in a certain order being sent to Upstart via D-Bus in the
reversed order. This could occur if the first one is ASYNC and the
second is SYNC. SYNC messages were being sent directly to D-Bus and
blocking the main thread until the response was received, whereas ASYNC
messages were posted to to the message loop queue where they would be
later sent to D-Bus. This caused the order in which messages were
delivered to D-Bus to not match developer's expectations.
With this change, the D-Bus messages are always sent synchronously and
block until they are acknowledged by Upstart, but the ASYNC events now
set the Upstart's wait flag to false to return as soon as the event is
queued. This causes the events to be delivered in the right order.
BUG=b:77528877
TEST=Reproduce conditions causing bug and ensure error isn't seen
Change-Id: I32e9d62a8c7ee86ff781890189d0a14c3be291ee
Reviewed-on: https://chromium-review.googlesource.com/1017700
Reviewed-by: Christopher Morin <cmtm@google.com>
Commit-Queue: Christopher Morin <cmtm@google.com>
Tested-by: Christopher Morin <cmtm@google.com>
Trybot-Ready: Christopher Morin <cmtm@google.com>
diff --git a/login_manager/upstart_signal_emitter.cc b/login_manager/upstart_signal_emitter.cc
index 3b2c856..61ceda2 100644
--- a/login_manager/upstart_signal_emitter.cc
+++ b/login_manager/upstart_signal_emitter.cc
@@ -38,20 +38,11 @@
dbus::MessageWriter writer(&method_call);
writer.AppendString(name);
writer.AppendArrayOfStrings(args_keyvals);
- writer.AppendBool(true);
-
- switch (mode) {
- case TriggerMode::SYNC:
- return upstart_dbus_proxy_->CallMethodAndBlock(
- &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
- case TriggerMode::ASYNC:
- upstart_dbus_proxy_->CallMethod(
- &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- dbus::ObjectProxy::EmptyResponseCallback());
- return nullptr;
- }
- NOTREACHED() << "Invalid trigger mode " << mode;
- return nullptr;
+ // When this boolean is true, Upstart waits until all side-effects of the
+ // event have completed instead of just returning after it's queued.
+ writer.AppendBool(mode == TriggerMode::SYNC);
+ return upstart_dbus_proxy_->CallMethodAndBlock(
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
}
} // namespace login_manager
