Initialize DBus in CreateInstance
We now expect the bus to be ready to go as soon as CreateInstance is
finished. The downside is that CreateInstance can now fail when the bus
isn't available, so we must handle nullptr returns.
Change-Id: I1e8d20f8d85d93e7e8b5e93ee1e5ba6043a347c4
Test: Verified commands continue to work
Bug: 26233663
diff --git a/client_library/client.cc b/client_library/client.cc
index d6e7382..f4a24cf 100644
--- a/client_library/client.cc
+++ b/client_library/client.cc
@@ -25,7 +25,14 @@
namespace update_engine {
std::unique_ptr<UpdateEngineClient> UpdateEngineClient::CreateInstance() {
- return unique_ptr<UpdateEngineClient>{new internal::UpdateEngineClientImpl{}};
+ auto update_engine_client_impl = new internal::UpdateEngineClientImpl{};
+ auto ret = unique_ptr<UpdateEngineClient>{update_engine_client_impl};
+
+ if (!update_engine_client_impl->Init()) {
+ ret.reset();
+ }
+
+ return ret;
}
} // namespace update_engine
diff --git a/client_library/client_impl.cc b/client_library/client_impl.cc
index 247f8f0..186301f 100644
--- a/client_library/client_impl.cc
+++ b/client_library/client_impl.cc
@@ -16,24 +16,30 @@
#include "update_engine/client_library/client_impl.h"
+#include <base/message_loop/message_loop.h>
+
#include <dbus/bus.h>
#include <update_engine/dbus-constants.h>
#include "update_engine/update_status_utils.h"
using chromeos_update_engine::StringToUpdateStatus;
+using std::string;
using dbus::Bus;
using org::chromium::UpdateEngineInterfaceProxy;
-using std::string;
namespace update_engine {
namespace internal {
-UpdateEngineClientImpl::UpdateEngineClientImpl() {
+bool UpdateEngineClientImpl::Init() {
Bus::Options options;
options.bus_type = Bus::SYSTEM;
scoped_refptr<Bus> bus{new Bus{options}};
+
+ if (!bus->Connect()) return false;
+
proxy_.reset(new UpdateEngineInterfaceProxy{bus});
+ return true;
}
bool UpdateEngineClientImpl::AttemptUpdate(const string& in_app_version,
@@ -48,7 +54,7 @@
double* out_progress,
UpdateStatus* out_update_status,
string* out_new_version,
- int64_t* out_new_size) {
+ int64_t* out_new_size) const {
string status_as_string;
const bool success =
proxy_->GetStatus(out_last_checked_time, out_progress, &status_as_string,
@@ -64,7 +70,7 @@
return proxy_->SetUpdateOverCellularPermission(allowed, nullptr);
}
-bool UpdateEngineClientImpl::GetUpdateOverCellularPermission(bool* allowed) {
+bool UpdateEngineClientImpl::GetUpdateOverCellularPermission(bool* allowed) const {
return proxy_->GetUpdateOverCellularPermission(allowed, nullptr);
}
@@ -72,7 +78,7 @@
return proxy_->SetP2PUpdatePermission(enabled, nullptr);
}
-bool UpdateEngineClientImpl::GetP2PUpdatePermission(bool* enabled) {
+bool UpdateEngineClientImpl::GetP2PUpdatePermission(bool* enabled) const {
return proxy_->GetP2PUpdatePermission(enabled, nullptr);
}
@@ -80,11 +86,11 @@
return proxy_->AttemptRollback(powerwash, nullptr);
}
-bool UpdateEngineClientImpl::GetRollbackPartition(string* rollback_partition) {
+bool UpdateEngineClientImpl::GetRollbackPartition(string* rollback_partition) const {
return proxy_->GetRollbackPartition(rollback_partition, nullptr);
}
-bool UpdateEngineClientImpl::GetPrevVersion(string* prev_version) {
+bool UpdateEngineClientImpl::GetPrevVersion(string* prev_version) const {
return proxy_->GetPrevVersion(prev_version, nullptr);
}
@@ -104,7 +110,7 @@
void UpdateEngineClientImpl::StatusUpdateHandlerRegistered(
StatusUpdateHandler* handler, const std::string& interface,
- const std::string& signal_name, bool success) {
+ const std::string& signal_name, bool success) const {
if (!success) {
handler->IPCError("Could not connect to" + signal_name);
return;
@@ -139,6 +145,11 @@
void UpdateEngineClientImpl::RegisterStatusUpdateHandler(
StatusUpdateHandler* handler) {
+ if (!base::MessageLoopForIO::current()) {
+ LOG(FATAL) << "Cannot get UpdateEngineClient outside of message loop.";
+ return;
+ }
+
proxy_->RegisterStatusUpdateSignalHandler(
base::Bind(&UpdateEngineClientImpl::RunStatusUpdateHandler,
base::Unretained(this), base::Unretained(handler)),
@@ -151,12 +162,12 @@
return proxy_->SetChannel(in_target_channel, allow_powerwash, nullptr);
}
-bool UpdateEngineClientImpl::GetTargetChannel(string* out_channel) {
+bool UpdateEngineClientImpl::GetTargetChannel(string* out_channel) const {
return proxy_->GetChannel(false, // Get the target channel.
out_channel, nullptr);
}
-bool UpdateEngineClientImpl::GetChannel(string* out_channel) {
+bool UpdateEngineClientImpl::GetChannel(string* out_channel) const {
return proxy_->GetChannel(true, // Get the current channel.
out_channel, nullptr);
}
diff --git a/client_library/client_impl.h b/client_library/client_impl.h
index 03c8f3c..cfd5395 100644
--- a/client_library/client_impl.h
+++ b/client_library/client_impl.h
@@ -31,7 +31,9 @@
class UpdateEngineClientImpl : public UpdateEngineClient {
public:
- UpdateEngineClientImpl();
+ explicit UpdateEngineClientImpl() = default;
+ bool Init();
+
virtual ~UpdateEngineClientImpl() = default;
bool AttemptUpdate(const std::string& app_version,
@@ -42,30 +44,30 @@
double* out_progress,
UpdateStatus* out_update_status,
std::string* out_new_version,
- int64_t* out_new_size) override;
+ int64_t* out_new_size) const override;
bool SetUpdateOverCellularPermission(bool allowed) override;
- bool GetUpdateOverCellularPermission(bool* allowed) override;
+ bool GetUpdateOverCellularPermission(bool* allowed) const override;
bool SetP2PUpdatePermission(bool enabled) override;
- bool GetP2PUpdatePermission(bool* enabled) override;
+ bool GetP2PUpdatePermission(bool* enabled) const override;
bool Rollback(bool powerwash) override;
- bool GetRollbackPartition(std::string* rollback_partition) override;
+ bool GetRollbackPartition(std::string* rollback_partition) const override;
void RebootIfNeeded() override;
- bool GetPrevVersion(std::string* prev_version) override;
+ bool GetPrevVersion(std::string* prev_version) const override;
bool ResetStatus() override;
bool SetTargetChannel(const std::string& target_channel,
bool allow_powerwash) override;
- bool GetTargetChannel(std::string* out_channel) override;
+ bool GetTargetChannel(std::string* out_channel) const override;
- bool GetChannel(std::string* out_channel) override;
+ bool GetChannel(std::string* out_channel) const override;
void RegisterStatusUpdateHandler(StatusUpdateHandler* handler) override;
@@ -75,7 +77,7 @@
void StatusUpdateHandlerRegistered(StatusUpdateHandler* handler,
const std::string& interface,
const std::string& signal_name,
- bool success);
+ bool success) const;
void RunStatusUpdateHandler(StatusUpdateHandler* handler,
int64_t last_checked_time,
diff --git a/client_library/include/update_engine/client.h b/client_library/include/update_engine/client.h
index 4d264c8..8bfb631 100644
--- a/client_library/include/update_engine/client.h
+++ b/client_library/include/update_engine/client.h
@@ -65,28 +65,28 @@
double* out_progress,
UpdateStatus* out_update_status,
std::string* out_new_version,
- int64_t* out_new_size) = 0;
+ int64_t* out_new_size) const = 0;
// Getter and setter for the updates over cellular connections.
virtual bool SetUpdateOverCellularPermission(bool allowed) = 0;
- virtual bool GetUpdateOverCellularPermission(bool* allowed) = 0;
+ virtual bool GetUpdateOverCellularPermission(bool* allowed) const = 0;
// Getter and setter for the updates from P2P permission.
virtual bool SetP2PUpdatePermission(bool enabled) = 0;
- virtual bool GetP2PUpdatePermission(bool* enabled) = 0;
+ virtual bool GetP2PUpdatePermission(bool* enabled) const = 0;
// Attempt a rollback. Set 'powerwash' to reset the device while rolling
// back.
virtual bool Rollback(bool powerwash) = 0;
// Get the rollback partition if available. Gives empty string if not.
- virtual bool GetRollbackPartition(std::string* rollback_partition) = 0;
+ virtual bool GetRollbackPartition(std::string* rollback_partition) const = 0;
// Reboot the system if needed.
virtual void RebootIfNeeded() = 0;
// Get the previous version
- virtual bool GetPrevVersion(std::string* prev_version) = 0;
+ virtual bool GetPrevVersion(std::string* prev_version) const = 0;
// Resets the status of the Update Engine
virtual bool ResetStatus() = 0;
@@ -96,10 +96,10 @@
bool allow_powerwash) = 0;
// Get the channel the device will switch to on reboot.
- virtual bool GetTargetChannel(std::string* out_channel) = 0;
+ virtual bool GetTargetChannel(std::string* out_channel) const = 0;
// Get the channel the device is currently on.
- virtual bool GetChannel(std::string* out_channel) = 0;
+ virtual bool GetChannel(std::string* out_channel) const = 0;
// Handle status updates. The handler must exist until the client is
// destroyed. Its IPCError method will be called if the handler could
diff --git a/update_engine_client.cc b/update_engine_client.cc
index b5c9c24..f062813 100644
--- a/update_engine_client.cc
+++ b/update_engine_client.cc
@@ -26,13 +26,10 @@
#include <base/command_line.h>
#include <base/logging.h>
#include <base/macros.h>
-#include <brillo/daemons/dbus_daemon.h>
+#include <brillo/daemons/daemon.h>
#include <brillo/flag_helper.h>
-#include <dbus/bus.h>
#include "update_engine/client.h"
-#include "update_engine/dbus-constants.h"
-#include "update_engine/dbus-proxies.h"
#include "update_engine/status_update_handler.h"
#include "update_engine/update_status.h"
#include "update_status_utils.h"
@@ -40,8 +37,6 @@
using std::string;
using std::unique_ptr;
using std::vector;
-using update_engine::kAttemptUpdateFlagNonInteractive;
-using update_engine::kUpdateEngineServiceName;
using update_engine::UpdateStatus;
using chromeos_update_engine::UpdateStatusToString;
@@ -51,38 +46,32 @@
// initialization.
const int kContinueRunning = -1;
-class UpdateEngineClient : public brillo::DBusDaemon {
+class UpdateEngineClient : public brillo::Daemon {
public:
UpdateEngineClient(int argc, char** argv) : argc_(argc), argv_(argv) {
- client_ = update_engine::UpdateEngineClient::CreateInstance();
}
~UpdateEngineClient() override = default;
protected:
int OnInit() override {
- int ret = DBusDaemon::OnInit();
+ int ret = Daemon::OnInit();
if (ret != EX_OK) return ret;
- if (!InitProxy()) return 1;
- // Wait for the UpdateEngine to be available or timeout.
- proxy_->GetObjectProxy()->WaitForServiceToBeAvailable(base::Bind(
- &UpdateEngineClient::OnServiceAvailable, base::Unretained(this)));
- base::MessageLoop::current()->PostDelayedTask(
- FROM_HERE, base::Bind(&UpdateEngineClient::OnServiceAvailableTimeout,
- base::Unretained(this)),
- base::TimeDelta::FromSeconds(10));
+
+ client_ = update_engine::UpdateEngineClient::CreateInstance();
+
+ if (!client_) {
+ LOG(ERROR) << "UpdateEngineService not available.";
+ return 1;
+ }
+
+ ret = ProcessFlags();
+ if (ret != kContinueRunning) QuitWithExitCode(ret);
+
return EX_OK;
}
private:
- bool InitProxy();
-
- // Callback called when the UpdateEngine service becomes available.
- void OnServiceAvailable(bool service_is_available);
-
- // Callback called when the UpdateEngine service doesn't become available
- // after a timeout.
- void OnServiceAvailableTimeout();
// Show the status of the update engine in stdout.
bool ShowStatus();
@@ -95,9 +84,6 @@
// flags.
int ProcessFlags();
- // DBus Proxy to the update_engine daemon object used for all the calls.
- std::unique_ptr<org::chromium::UpdateEngineInterfaceProxy> proxy_;
-
// Copy of argc and argv passed to main().
int argc_;
char** argv_;
@@ -114,34 +100,6 @@
DISALLOW_COPY_AND_ASSIGN(UpdateEngineClient);
};
-bool UpdateEngineClient::InitProxy() {
- proxy_.reset(new org::chromium::UpdateEngineInterfaceProxy(bus_));
-
- if (!proxy_->GetObjectProxy()) {
- LOG(ERROR) << "Error getting dbus proxy for " << kUpdateEngineServiceName;
- return false;
- }
- return true;
-}
-
-void UpdateEngineClient::OnServiceAvailable(bool service_is_available) {
- service_is_available_ = service_is_available;
- if (!service_is_available) {
- LOG(ERROR) << "UpdateEngineService not available.";
- QuitWithExitCode(-1);
- }
- int ret = ProcessFlags();
- if (ret != kContinueRunning) QuitWithExitCode(ret);
-}
-
-void UpdateEngineClient::OnServiceAvailableTimeout() {
- if (!service_is_available_) {
- LOG(ERROR) << "Waiting for UpdateEngineService timeout. Is update_engine "
- "daemon running?";
- QuitWithExitCode(-1);
- }
-}
-
class ExitingStatusUpdateHandler : public update_engine::StatusUpdateHandler {
public:
~ExitingStatusUpdateHandler() override = default;