diff options
| -rw-r--r-- | dbus/bus.h | 4 | ||||
| -rw-r--r-- | dbus/exported_object.h | 8 | ||||
| -rw-r--r-- | dbus/test_service.cc | 25 | ||||
| -rw-r--r-- | dbus/test_service.h | 2 |
4 files changed, 27 insertions, 12 deletions
@@ -421,6 +421,10 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> { // Requests the ownership of the given service name. // Returns true on success, or the the service name is already obtained. // + // Note that it's important to expose methods before requesting a service + // name with this method. See also ExportedObject::ExportMethodAndBlock() + // for details. + // // BLOCKING CALL. virtual bool RequestOwnershipAndBlock(const std::string& service_name, ServiceOwnershipOptions options); diff --git a/dbus/exported_object.h b/dbus/exported_object.h index eae2767..5f74dc2 100644 --- a/dbus/exported_object.h +++ b/dbus/exported_object.h @@ -66,6 +66,14 @@ class CHROME_DBUS_EXPORT ExportedObject // |method_callback| can safely reference objects in the origin thread // (i.e. UI thread in most cases). // + // IMPORTANT NOTE: You should export all methods before requesting a + // service name by Bus::RequestOwnership/AndBlock(). If you do it in the + // wrong order (i.e. request a service name then export methods), there + // will be a short time period where your service is unable to respond to + // method calls because these methods aren't yet exposed. This race is a + // real problem as clients may start calling methods of your service as + // soon as you acquire a service name, by watching the name owner change. + // // BLOCKING CALL. virtual bool ExportMethodAndBlock(const std::string& interface_name, const std::string& method_name, diff --git a/dbus/test_service.cc b/dbus/test_service.cc index 5f59501..21a885d 100644 --- a/dbus/test_service.cc +++ b/dbus/test_service.cc @@ -38,7 +38,7 @@ TestService::TestService(const Options& options) : base::Thread("TestService"), request_ownership_options_(options.request_ownership_options), dbus_task_runner_(options.dbus_task_runner), - on_all_methods_exported_(false, false), + on_name_obtained_(false, false), num_exported_methods_(0) { } @@ -54,8 +54,8 @@ bool TestService::StartService() { bool TestService::WaitUntilServiceIsStarted() { const base::TimeDelta timeout(TestTimeouts::action_max_timeout()); - // Wait until all methods are exported. - return on_all_methods_exported_.TimedWait(timeout); + // Wait until the ownership of the service name is obtained. + return on_name_obtained_.TimedWait(timeout); } void TestService::ShutdownAndBlock() { @@ -138,6 +138,8 @@ void TestService::OnOwnership(base::Callback<void(bool)> callback, has_ownership_ = success; LOG_IF(ERROR, !success) << "Failed to own: " << service_name; callback.Run(success); + + on_name_obtained_.Signal(); } void TestService::OnExported(const std::string& interface_name, @@ -152,8 +154,15 @@ void TestService::OnExported(const std::string& interface_name, } ++num_exported_methods_; - if (num_exported_methods_ == kNumMethodsToExport) - on_all_methods_exported_.Signal(); + if (num_exported_methods_ == kNumMethodsToExport) { + // As documented in exported_object.h, the service name should be + // requested after all methods are exposed. + bus_->RequestOwnership("org.chromium.TestService", + request_ownership_options_, + base::Bind(&TestService::OnOwnership, + base::Unretained(this), + base::Bind(&EmptyCallback))); + } } void TestService::Run(base::MessageLoop* message_loop) { @@ -163,12 +172,6 @@ void TestService::Run(base::MessageLoop* message_loop) { bus_options.dbus_task_runner = dbus_task_runner_; bus_ = new Bus(bus_options); - bus_->RequestOwnership("org.chromium.TestService", - request_ownership_options_, - base::Bind(&TestService::OnOwnership, - base::Unretained(this), - base::Bind(&EmptyCallback))); - exported_object_ = bus_->GetExportedObject( ObjectPath("/org/chromium/TestObject")); diff --git a/dbus/test_service.h b/dbus/test_service.h index 7ddaf21..197b8d2 100644 --- a/dbus/test_service.h +++ b/dbus/test_service.h @@ -170,7 +170,7 @@ class TestService : public base::Thread { Bus::ServiceOwnershipOptions request_ownership_options_; scoped_refptr<base::SequencedTaskRunner> dbus_task_runner_; - base::WaitableEvent on_all_methods_exported_; + base::WaitableEvent on_name_obtained_; // The number of methods actually exported. int num_exported_methods_; |