Browse Source

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.
Charles Kerr 6 years ago
parent
commit
eb8546c8d1
2 changed files with 32 additions and 35 deletions
  1. 32 34
      atom/browser/lib/power_observer_linux.cc
  2. 0 1
      atom/browser/lib/power_observer_linux.h

+ 32 - 34
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<bool()> 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;

+ 0 - 1
atom/browser/lib/power_observer_linux.h

@@ -40,7 +40,6 @@ class PowerObserverLinux : public base::PowerObserver {
 
   base::Callback<bool()> should_shutdown_;
 
-  scoped_refptr<dbus::Bus> bus_;
   scoped_refptr<dbus::ObjectProxy> logind_;
   std::string lock_owner_name_;
   base::ScopedFD sleep_lock_;