Fix MessagePumpFuchsia to cope with FDWatchers which outlive it.

The Fuchsia FileDescriptorWatcher attempted to cancel the wait operation
on deletion, failing if the pump was already deleted because the
underlying wait port was no longer valid.

FileDescriptorWatcher now access the wait port handle via a WeakPtr to
the associated MessagePumpFuchsia, allowing it to skip cancelling the
operation in this case.

Bug: 738275
Change-Id: Icd2a8811219de182c9c9a3208224e46a9ce1d186
Reviewed-on: https://chromium-review.googlesource.com/575291
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487314}

base/message_loop/message_pump_fuchsia.cc

@@ -28,22 +28,23 @@ MessagePumpFuchsia::FileDescriptorWatcher::~FileDescriptorWatcher() {
 }
 
 bool MessagePumpFuchsia::FileDescriptorWatcher::StopWatchingFileDescriptor() {
-  if (handle_ == MX_HANDLE_INVALID)
+  if (!weak_pump_ || handle_ == MX_HANDLE_INVALID)
     return true;
-  uint64_t this_as_key =
-      static_cast<uint64_t>(reinterpret_cast<uintptr_t>(this));
-  int result = mx_port_cancel(port_, handle_, this_as_key);
+
+  int result = mx_port_cancel(weak_pump_->port_, handle_, wait_key());
   DLOG_IF(ERROR, result != MX_OK)
       << "mx_port_cancel(handle=" << handle_
       << ") failed: " << mx_status_get_string(result);
   handle_ = MX_HANDLE_INVALID;
+
   return result == MX_OK;
 }
 
-MessagePumpFuchsia::MessagePumpFuchsia() : keep_running_(true) {
+MessagePumpFuchsia::MessagePumpFuchsia()
+    : keep_running_(true), weak_factory_(this) {
   // TODO(wez): Remove MX_PORT_OPT_V2 once the SDK is rolled, or migrate
   // this implementation use ulib/port helpers.
-  CHECK(mx_port_create(MX_PORT_OPT_V2, &port_) == MX_OK);
+  CHECK_EQ(0, mx_port_create(MX_PORT_OPT_V2, &port_));
 }
 
 MessagePumpFuchsia::~MessagePumpFuchsia() {
@@ -61,8 +62,10 @@ bool MessagePumpFuchsia::WatchFileDescriptor(int fd,
   DCHECK_GE(fd, 0);
   DCHECK(controller);
   DCHECK(delegate);
+
+  controller->fd_ = fd;
+  controller->persistent_ = persistent;
   controller->watcher_ = delegate;
-  controller->port_ = port_;
 
   uint32_t events = 0;
   switch (mode) {
@@ -79,7 +82,6 @@ bool MessagePumpFuchsia::WatchFileDescriptor(int fd,
       NOTREACHED() << "unexpected mode: " << mode;
       return false;
   }
-
   controller->desired_events_ = events;
 
   controller->io_ = __mxio_fd_to_io(fd);
@@ -88,8 +90,7 @@ bool MessagePumpFuchsia::WatchFileDescriptor(int fd,
     return false;
   }
 
-  controller->fd_ = fd;
-  controller->persistent_ = persistent;
+  controller->weak_pump_ = weak_factory_.GetWeakPtr();
 
   return controller->WaitBegin();
 }
@@ -102,13 +103,12 @@ bool MessagePumpFuchsia::FileDescriptorWatcher::WaitBegin() {
     return false;
   }
 
-  uint64_t this_as_key =
-      static_cast<uint64_t>(reinterpret_cast<uintptr_t>(this));
-  mx_status_t status = mx_object_wait_async(handle_, port_, this_as_key,
-                                            signals, MX_WAIT_ASYNC_ONCE);
+  mx_status_t status = mx_object_wait_async(
+      handle_, weak_pump_->port_, wait_key(), signals, MX_WAIT_ASYNC_ONCE);
   if (status != MX_OK) {
     DLOG(ERROR) << "mx_object_wait_async failed: "
-                << mx_status_get_string(status) << " (port=" << port_ << ")";
+                << mx_status_get_string(status)
+                << " (port=" << weak_pump_->port_ << ")";
     return false;
   }
   return true;

base/message_loop/message_pump_fuchsia.h

@@ -8,6 +8,7 @@
 #include "base/base_export.h"
 #include "base/location.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_pump.h"
 
 #include <magenta/syscalls/port.h>
@@ -44,20 +45,24 @@ class BASE_EXPORT MessagePumpFuchsia : public MessagePump {
     }
 
    private:
+    friend class MessagePumpFuchsia;
+
     // Start watching the FD.
     bool WaitBegin();
 
     // Stop watching the FD. Returns the set of events the watcher is interested
     // in based on the observed bits from the underlying packet.
     uint32_t WaitEnd(uint32_t observed);
 
-    friend class MessagePumpFuchsia;
+    // Returns the key to use to uniquely identify this object's wait operation.
+    uint64_t wait_key() const {
+      return static_cast<uint64_t>(reinterpret_cast<uintptr_t>(this));
+    }
 
     const tracked_objects::Location created_from_location_;
 
     // Set directly from the inputs to WatchFileDescriptor.
     Watcher* watcher_ = nullptr;
-    mx_handle_t port_ = MX_HANDLE_INVALID;
     int fd_ = -1;
     uint32_t desired_events_ = 0;
 
@@ -68,6 +73,9 @@ class BASE_EXPORT MessagePumpFuchsia : public MessagePump {
     // WaitBegin and WaitEnd calls), and MX_HANDLE_INVALID otherwise.
     mx_handle_t handle_ = MX_HANDLE_INVALID;
 
+    // Used to safely access resources owned by the associated message pump.
+    WeakPtr<MessagePumpFuchsia> weak_pump_;
+
     // This bool is used during calling |Watcher| callbacks. This object's
     // lifetime is owned by the user of this class. If the message loop is woken
     // up in the case where it needs to call both the readable and writable
@@ -113,6 +121,8 @@ class BASE_EXPORT MessagePumpFuchsia : public MessagePump {
   // The time at which we should call DoDelayedWork.
   TimeTicks delayed_work_time_;
 
+  base::WeakPtrFactory<MessagePumpFuchsia> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(MessagePumpFuchsia);
 };