From eb8546c8d165b6a528447aa3a410e9e790b06147 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 12 Oct 2018 00:40:22 -0500 Subject: [PATCH] fix: check dbus response for null before use. (#15033) * fix: backport #15030 to fix #14958 dbus crash * chore: re-enable power spec tests * chore: undo changes made to power monitor tests. The Linux failures on that are gone in master / 4-0-x. Whatever resolved it there is unrelated to this PR's changes, so I'm not going to block this fix on an unrelated issue. --- atom/browser/lib/power_observer_linux.cc | 66 ++++++++++++------------ atom/browser/lib/power_observer_linux.h | 1 - 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/atom/browser/lib/power_observer_linux.cc b/atom/browser/lib/power_observer_linux.cc index 1719fad2dadf..cd7cba23ebb8 100644 --- a/atom/browser/lib/power_observer_linux.cc +++ b/atom/browser/lib/power_observer_linux.cc @@ -32,21 +32,28 @@ namespace atom { PowerObserverLinux::PowerObserverLinux() : lock_owner_name_(get_executable_basename()), weak_ptr_factory_(this) { - auto* dbus_thread_manager = bluez::DBusThreadManagerLinux::Get(); - if (dbus_thread_manager) { - bus_ = dbus_thread_manager->GetSystemBus(); - if (bus_) { - logind_ = bus_->GetObjectProxy(kLogindServiceName, - dbus::ObjectPath(kLogindObjectPath)); - logind_->WaitForServiceToBeAvailable( - base::Bind(&PowerObserverLinux::OnLoginServiceAvailable, - weak_ptr_factory_.GetWeakPtr())); - } else { - LOG(WARNING) << "Failed to get system bus connection"; - } - } else { - LOG(WARNING) << "DBusThreadManagerLinux instance isn't available"; + auto* bus = bluez::DBusThreadManagerLinux::Get()->GetSystemBus(); + if (!bus) { + LOG(WARNING) << "Failed to get system bus connection"; + return; } + + // set up the logind proxy + + const auto weakThis = weak_ptr_factory_.GetWeakPtr(); + + logind_ = bus->GetObjectProxy(kLogindServiceName, + dbus::ObjectPath(kLogindObjectPath)); + logind_->ConnectToSignal( + kLogindManagerInterface, "PrepareForShutdown", + base::BindRepeating(&PowerObserverLinux::OnPrepareForShutdown, weakThis), + base::BindRepeating(&PowerObserverLinux::OnSignalConnected, weakThis)); + logind_->ConnectToSignal( + kLogindManagerInterface, "PrepareForSleep", + base::BindRepeating(&PowerObserverLinux::OnPrepareForSleep, weakThis), + base::BindRepeating(&PowerObserverLinux::OnSignalConnected, weakThis)); + logind_->WaitForServiceToBeAvailable(base::BindRepeating( + &PowerObserverLinux::OnLoginServiceAvailable, weakThis)); } PowerObserverLinux::~PowerObserverLinux() = default; @@ -56,17 +63,6 @@ void PowerObserverLinux::OnLoginServiceAvailable(bool service_available) { LOG(WARNING) << kLogindServiceName << " not available"; return; } - // Connect to PrepareForShutdown/PrepareForSleep signals - logind_->ConnectToSignal(kLogindManagerInterface, "PrepareForShutdown", - base::Bind(&PowerObserverLinux::OnPrepareForShutdown, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&PowerObserverLinux::OnSignalConnected, - weak_ptr_factory_.GetWeakPtr())); - logind_->ConnectToSignal(kLogindManagerInterface, "PrepareForSleep", - base::Bind(&PowerObserverLinux::OnPrepareForSleep, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&PowerObserverLinux::OnSignalConnected, - weak_ptr_factory_.GetWeakPtr())); // Take sleep inhibit lock BlockSleep(); } @@ -80,10 +76,10 @@ void PowerObserverLinux::BlockSleep() { inhibit_writer.AppendString(lock_owner_name_); // who inhibit_writer.AppendString("Application cleanup before suspend"); // why inhibit_writer.AppendString("delay"); // mode - logind_->CallMethod(&sleep_inhibit_call, - dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, - base::Bind(&PowerObserverLinux::OnInhibitResponse, - weak_ptr_factory_.GetWeakPtr(), &sleep_lock_)); + logind_->CallMethod( + &sleep_inhibit_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, + base::BindOnce(&PowerObserverLinux::OnInhibitResponse, + weak_ptr_factory_.GetWeakPtr(), &sleep_lock_)); } void PowerObserverLinux::UnblockSleep() { @@ -103,8 +99,8 @@ void PowerObserverLinux::BlockShutdown() { inhibit_writer.AppendString("delay"); // mode logind_->CallMethod( &shutdown_inhibit_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, - base::Bind(&PowerObserverLinux::OnInhibitResponse, - weak_ptr_factory_.GetWeakPtr(), &shutdown_lock_)); + base::BindOnce(&PowerObserverLinux::OnInhibitResponse, + weak_ptr_factory_.GetWeakPtr(), &shutdown_lock_)); } void PowerObserverLinux::UnblockShutdown() { @@ -122,8 +118,10 @@ void PowerObserverLinux::SetShutdownHandler(base::Callback handler) { void PowerObserverLinux::OnInhibitResponse(base::ScopedFD* scoped_fd, dbus::Response* response) { - dbus::MessageReader reader(response); - reader.PopFileDescriptor(scoped_fd); + if (response != nullptr) { + dbus::MessageReader reader(response); + reader.PopFileDescriptor(scoped_fd); + } } void PowerObserverLinux::OnPrepareForSleep(dbus::Signal* signal) { @@ -158,7 +156,7 @@ void PowerObserverLinux::OnPrepareForShutdown(dbus::Signal* signal) { } } -void PowerObserverLinux::OnSignalConnected(const std::string& interface, +void PowerObserverLinux::OnSignalConnected(const std::string& /*interface*/, const std::string& signal, bool success) { LOG_IF(WARNING, !success) << "Failed to connect to " << signal; diff --git a/atom/browser/lib/power_observer_linux.h b/atom/browser/lib/power_observer_linux.h index d7ca6557ce1b..f3173e600159 100644 --- a/atom/browser/lib/power_observer_linux.h +++ b/atom/browser/lib/power_observer_linux.h @@ -40,7 +40,6 @@ class PowerObserverLinux : public base::PowerObserver { base::Callback should_shutdown_; - scoped_refptr bus_; scoped_refptr logind_; std::string lock_owner_name_; base::ScopedFD sleep_lock_;