dbus-c++: add acquire_name method to Connection class
Our daemons would like to acquire a D-Bus name, or exit if the
name is not available. This will guarantee that at most one instance
of a daemon is running at any given time. (More precisely: the
guarantee will be that no more than one process of each daemon has
continued past its D-Bus setup code.)
Unfortunately, the existing request_name method doesn't tell
us whether or not our request succeeded. Add a new method that
does.
If our request_name attempt fails, the new method also pings
the current owner of the name. This updates dbus-daemon's
knowledge of whether or not that owner is alive. After this,
we retry request_name once.
The ping and retry should avoid a potential race, where a
name owner has terminated abruptly, and our name request
is processed by dbus-daemon before it realizes the name
owner has exited.
BUG=chromium:356874
TEST=network_TwoShills, manual
Manual test
-----------
- build shill with this patch and CL:195012
- install the new shill and dbus-c++
// when the old shill answers the d-bus ping, the new shill
// aborts.
dut# pid=$(pgrep shill)
dut# (kill -STOP $pid; sleep 10; kill -CONT $pid) & (time /usr/bin/shill --foreground; /usr/local/lib/flimflam/test/list-active-service | head -1)
[...]
[0416/151422:FATAL:dbus_control.cc(94)] Failed to acquire D-Bus name org.chromium.flimflam. Is another shill running?
[...]
real 0m10.041s
user 0m0.008s
sys 0m0.013s
[ /service/1 ]
// if the old shill times out the d-bus ping, the new shill
// still aborts. (the old shill may be hung, but we should
// deal with that some other way.)
dut# kill -STOP $pid & (time /usr/bin/shill --foreground; kill -CONT $pid; /usr/local/lib/flimflam/test/list-active-service | head -1)
[...]
[0416/151554:FATAL:dbus_control.cc(94)] Failed to acquire D-Bus name org.chromium.flimflam. Is another shill running?
real 0m25.086s
user 0m0.015s
sys 0m0.008s
[ /service/1 ]
// if the old shill dies in the middle of the ping, the new
// shill takes over.
Wed Apr 16 16:13:42 PDT 2014
2014-04-16T16:13:42.100291-07:00 localhost shill: [0416/161342:INFO:manager.cc(197)] Manager started.
[ /service/1 ]
Change-Id: I95b10747750bd0c983ca91b9f4fbaf2c8ed3b8cf
Reviewed-on: https://chromium-review.googlesource.com/195002
Tested-by: mukesh agrawal <[email protected]>
Reviewed-by: Ben Chan <[email protected]>
Commit-Queue: mukesh agrawal <[email protected]>
diff --git a/include/dbus-c++/connection.h b/include/dbus-c++/connection.h
index 7c19ea1..7db166f 100644
--- a/include/dbus-c++/connection.h
+++ b/include/dbus-c++/connection.h
@@ -416,8 +416,30 @@
*/
PendingCall *send_async(Message& msg, int timeout = -1);
+ /*!
+ *\brief Request a name from DBus.
+ *
+ * Request ownership of a name. The name and flags are passed
+ * on to the lower-layer.
+ *
+ * Unless you are doing something unusual, you should use
+ * acquire_name instead of this method.
+ *
+ * \param name The name to request ownership of.
+ * \param flags Flags to be passed to dbus_bus_request_name.
+ */
void request_name( const char* name, int flags = 0 );
-
+
+ /*!
+ *\brief Acquire a name from DBus.
+ *
+ * Request ownership of a name. Return true if the name was
+ * acquired, and false otherwise.
+ *
+ * \return True if the name was acquired, and false otherwise.
+ */
+ bool acquire_name( const char* name );
+
unsigned long sender_unix_uid(const char *sender);
/*!
@@ -462,6 +484,7 @@
private:
DXXAPILOCAL void init();
+ DXXAPILOCAL int _request_name( const char* name, int flags );
private:
diff --git a/src/connection.cpp b/src/connection.cpp
index 9b5bc59..10d96bd 100644
--- a/src/connection.cpp
+++ b/src/connection.cpp
@@ -388,17 +388,12 @@
return new PendingCall(new PendingCall::Private(pending));
}
-void Connection::request_name(const char *name, int flags)
+int Connection::_request_name(const char *name, int flags)
{
InternalError e;
debug_log("%s: registering bus name %s", unique_name(), name);
- /*
- * TODO:
- * Think about giving back the 'ret' value. Some people on the list
- * requested about this...
- */
int ret = dbus_bus_request_name(_pvt->conn, name, flags, e);
if (ret == -1)
@@ -414,6 +409,75 @@
std::string match = "destination='" + _pvt->names.back() + "'";
add_match(match.c_str());
}
+
+ return ret;
+}
+
+void Connection::request_name(const char *name, int flags)
+{
+ /*
+ * TODO:
+ * Think about giving back the 'ret' value. Some people on the list
+ * requested about this...
+ */
+ _request_name(name, flags);
+}
+
+bool Connection::acquire_name(const char *name)
+{
+ /*
+ * Request the desired name, allowing the request to be queued.
+ */
+ int ret = _request_name(name, 0);
+ switch (ret) {
+ case DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER:
+ case DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER:
+ return true;
+ case DBUS_REQUEST_NAME_REPLY_IN_QUEUE:
+ break;
+ case DBUS_REQUEST_NAME_REPLY_EXISTS:
+ default:
+ // Unexpected, but try to keep going.
+ debug_log("%s: unexpected reply %d for RequestName",
+ dbus_bus_get_unique_name(_pvt->conn), ret);
+ break;
+ }
+
+ /*
+ * We didn't get the name. Perhaps the name owner terminated
+ * abruptly, and dbus-daemon doesn't know that yet. Ask
+ * dbus-daemon to check if the name owner is still up.
+ */
+ CallMessage ping_request(name, "/", "org.freedesktop.DBus.Peer", "Ping");
+ try {
+ send_blocking(ping_request, DBUS_TIMEOUT_USE_DEFAULT);
+ } catch (const DBus::Error &) {
+ /*
+ * We don't care if the ping request times out or generates
+ * some other error. We only care about its side-effect.
+ */
+ }
+
+ /*
+ * Now that we've nudged dbus-daemon, try requesting the name
+ * one more time. Note that we may already be the owner at
+ * this point, because our first attempt allowed queuing the
+ * request.
+ */
+ ret = _request_name(name, DBUS_NAME_FLAG_DO_NOT_QUEUE);
+ switch (ret) {
+ case DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER:
+ case DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER:
+ return true;
+ case DBUS_REQUEST_NAME_REPLY_EXISTS:
+ return false;
+ case DBUS_REQUEST_NAME_REPLY_IN_QUEUE:
+ default:
+ // Log unexpected response.
+ debug_log("%s: unexpected reply %d for RequestName",
+ dbus_bus_get_unique_name(_pvt->conn), ret);
+ return false;
+ }
}
unsigned long Connection::sender_unix_uid(const char *sender)
