[fuchsia] Replace use of epoll with mx_ primitives in process_fuchsia

runtime/bin/process_fuchsia.cc

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #if !defined(DART_IO_DISABLED)
 
 #include "platform/globals.h"
 #if defined(HOST_OS_FUCHSIA)
 
 #include "bin/process.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <launchpad/launchpad.h>
 #include <launchpad/vmo.h>
 #include <magenta/process.h>
 #include <magenta/status.h>
 #include <magenta/syscalls.h>
 #include <magenta/syscalls/object.h>
+#include <magenta/types.h>
+#include <mxio/private.h>
 #include <mxio/util.h>
 #include <pthread.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/epoll.h>
 #include <unistd.h>
 
 #include "bin/dartutils.h"
@@ skipping 40 matching lines @@
   void set_next(ProcessInfo* info) { next_ = info; }
 
  private:
   mx_handle_t process_;
   intptr_t exit_pipe_fd_;
   ProcessInfo* next_;
 
   DISALLOW_COPY_AND_ASSIGN(ProcessInfo);
 };
 
-
 // Singly-linked list of ProcessInfo objects for all active processes
 // started from Dart.
 class ProcessInfoList {
  public:
   static void AddProcess(mx_handle_t process, intptr_t fd) {
     MutexLocker locker(mutex_);
     ProcessInfo* info = new ProcessInfo(process, fd);
     info->set_next(active_processes_);
     active_processes_ = info;
   }
@@ skipping 327 matching lines @@
 
 bool ExitCodeHandler::do_shutdown_ = false;
 bool ExitCodeHandler::running_ = false;
 bool ExitCodeHandler::terminate_done_ = false;
 Monitor* ExitCodeHandler::monitor_ = new Monitor();
 
 void Process::TerminateExitCodeHandler() {
   ExitCodeHandler::Terminate();
 }
 
-

[zra, 2017/05/26 16:13:22]

Dart VM style is two newlines between top-level functions in .cc files.

[jamesr1, 2017/05/27 00:14:09]

Turns out I wasn't reading your .clang-format file, fixed!

 intptr_t Process::CurrentProcessId() {
   return static_cast<intptr_t>(getpid());
 }
 
-
 int64_t Process::CurrentRSS() {
   mx_info_task_stats_t task_stats;
   mx_handle_t process = mx_process_self();
   mx_status_t status = mx_object_get_info(
       process, MX_INFO_TASK_STATS, &task_stats, sizeof(task_stats), NULL, NULL);
   if (status != NO_ERROR) {
     // TODO(zra): Translate this to a Unix errno.
     errno = status;
     return -1;
   }
   return task_stats.mem_committed_bytes;
 }
 
-
 int64_t Process::MaxRSS() {
   // There is currently no way to get the high watermark value on Fuchsia, so
   // just return the current RSS value.
   return CurrentRSS();
 }
 
-
 static bool ProcessWaitCleanup(intptr_t out,
                                intptr_t err,
-                               intptr_t exit_event,
-                               intptr_t epoll_fd) {
+                               intptr_t exit_event) {
   int e = errno;
   VOID_NO_RETRY_EXPECTED(close(out));
   VOID_NO_RETRY_EXPECTED(close(err));
   VOID_NO_RETRY_EXPECTED(close(exit_event));
-  VOID_NO_RETRY_EXPECTED(close(epoll_fd));
   errno = e;
   return false;
 }
 
+class MxioWaitEntry {
+ public:
+  explicit MxioWaitEntry(int fd) : mxio_(__mxio_fd_to_io(fd)) {}
+  ~MxioWaitEntry() { Cancel(); }
+
+  void WaitBegin(mx_wait_item_t* wait_item) {
+    if (mxio_ == nullptr) {

[zra, 2017/05/26 16:13:22]

To be consistent with the rest of the VM, please use NULL instead of nullptr.

[jamesr1, 2017/05/27 00:14:10]

done

+      *wait_item = {};
+      return;
+    }
+
+    __mxio_wait_begin(mxio_, EPOLLRDHUP | EPOLLIN, &wait_item->handle,
+                      &wait_item->waitfor);
+    wait_item->pending = 0;
+  }
+
+  void WaitEnd(mx_wait_item_t* wait_item, uint32_t* event) {
+    if (mxio_ == nullptr) {
+      *event = 0;
+      return;
+    }
+    __mxio_wait_end(mxio_, wait_item->pending, event);
+  }
+
+  void Cancel() {
+    if (mxio_)

[zra, 2017/05/26 16:13:22]

if (mxio_ != NULL)

Also, please use curly braces around single line if.

[jamesr1, 2017/05/27 00:14:10]

done

+      __mxio_release(mxio_);
+    mxio_ = nullptr;
+  }
+
+ private:
+  mxio_t* mxio_;
+};

[zra, 2017/05/26 16:13:22]

Maybe DISALLOW_COPY_AND_ASSIGN(MxioWaitEntry) or DISALLOW_ALLOCATION().

[jamesr1, 2017/05/27 00:14:10]

done

 
 bool Process::Wait(intptr_t pid,
                    intptr_t in,
                    intptr_t out,
                    intptr_t err,
                    intptr_t exit_event,
                    ProcessResult* result) {
   VOID_NO_RETRY_EXPECTED(close(in));
 
   // There is no return from this function using Dart_PropagateError
   // as memory used by the buffer lists is freed through their
   // destructors.
   BufferList out_data;
   BufferList err_data;
   union {
     uint8_t bytes[8];
     int32_t ints[2];
   } exit_code_data;
 
-  // The initial size passed to epoll_create is ignore on newer (>=
-  // 2.6.8) Linux versions
-  static const int kEpollInitialSize = 64;
-  int epoll_fd = NO_RETRY_EXPECTED(epoll_create(kEpollInitialSize));
-  if (epoll_fd == -1) {
-    return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-  }
-  if (!FDUtils::SetCloseOnExec(epoll_fd)) {
-    return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-  }
+  uint32_t events[3];
+  mx_wait_item_t wait_items[3];

[zra, 2017/05/26 16:13:22]

const intptr_t kWaitItemsCount = 3;

[jamesr1, 2017/05/27 00:14:10]

done

+  size_t active = 3;
 
-  struct epoll_event event;
-  event.events = EPOLLRDHUP | EPOLLIN;
-  event.data.fd = out;
-  int status = NO_RETRY_EXPECTED(
-      epoll_ctl(epoll_fd, EPOLL_CTL_ADD, out, &event));
-  if (status == -1) {
-    return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-  }
-  event.data.fd = err;
-  status = NO_RETRY_EXPECTED(
-      epoll_ctl(epoll_fd, EPOLL_CTL_ADD, err, &event));
-  if (status == -1) {
-    return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-  }
-  event.data.fd = exit_event;
-  status = NO_RETRY_EXPECTED(
-      epoll_ctl(epoll_fd, EPOLL_CTL_ADD, exit_event, &event));
-  if (status == -1) {
-    return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-  }
-  intptr_t active = 3;
+  MxioWaitEntry entries[3] = {
+      MxioWaitEntry(out), MxioWaitEntry(err), MxioWaitEntry(exit_event),
+  };
 
-  static const intptr_t kMaxEvents = 16;
-  struct epoll_event events[kMaxEvents];
   while (active > 0) {
     // TODO(US-109): When the epoll implementation is properly edge-triggered,
     // remove this sleep, which prevents the message queue from being
     // overwhelmed and leading to memory exhaustion.
     usleep(5000);

[zra, 2017/05/26 16:13:22]

This was copy-pasted from the eventhandler, but probably wasn't needed here in
the first place since the read()s from the ready fds were happening on the same
thread. This is safe to remove.

[jamesr1, 2017/05/27 00:14:10]

done

-    intptr_t result = NO_RETRY_EXPECTED(
-        epoll_wait(epoll_fd, events, kMaxEvents, -1));
-    if ((result < 0) && (errno != EWOULDBLOCK)) {
-      return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
+
+    for (size_t i = 0; i < 3; ++i) {
+      entries[i].WaitBegin(&wait_items[i]);
     }
-    for (intptr_t i = 0; i < result; i++) {
-      if ((events[i].events & EPOLLIN) != 0) {
-        const intptr_t avail = FDUtils::AvailableBytes(events[i].data.fd);
-        if (events[i].data.fd == out) {
-          if (!out_data.Read(out, avail)) {
-            return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-          }
-        } else if (events[i].data.fd == err) {
-          if (!err_data.Read(err, avail)) {
-            return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-          }
-        } else if (events[i].data.fd == exit_event) {
-          if (avail == 8) {
-            intptr_t b =
-                NO_RETRY_EXPECTED(read(exit_event, exit_code_data.bytes, 8));
-            if (b != 8) {
-              return ProcessWaitCleanup(out, err, exit_event, epoll_fd);
-            }
-          }
-        } else {
-          UNREACHABLE();
+    mx_object_wait_many(wait_items, 3, MX_TIME_INFINITE);
+
+    for (size_t i = 0; i < 3; ++i) {
+      entries[i].WaitEnd(&wait_items[i], &events[i]);
+    }
+
+    if ((events[0] & EPOLLIN) != 0) {
+      const intptr_t avail = FDUtils::AvailableBytes(out);
+      if (!out_data.Read(out, avail)) {
+        return ProcessWaitCleanup(out, err, exit_event);
+      }
+    }
+    if ((events[1] & EPOLLIN) != 0) {
+      const intptr_t avail = FDUtils::AvailableBytes(err);
+      if (!err_data.Read(err, avail)) {
+        return ProcessWaitCleanup(err, err, exit_event);
+      }
+    }
+    if ((events[2] & EPOLLIN) != 0) {
+      const intptr_t avail = FDUtils::AvailableBytes(exit_event);
+      if (avail == 8) {
+        intptr_t b =
+            NO_RETRY_EXPECTED(read(exit_event, exit_code_data.bytes, 8));
+        if (b != 8) {
+          return ProcessWaitCleanup(out, err, exit_event);
         }
       }
-      if ((events[i].events & (EPOLLHUP | EPOLLRDHUP)) != 0) {
-        NO_RETRY_EXPECTED(close(events[i].data.fd));
+    }
+    for (size_t i = 0; i < 3; ++i) {
+      if ((events[i] & EPOLLRDHUP) != 0) {
         active--;
-        VOID_NO_RETRY_EXPECTED(
-            epoll_ctl(epoll_fd, EPOLL_CTL_DEL, events[i].data.fd, NULL));
+        entries[i].Cancel();
       }
     }
   }
-  VOID_NO_RETRY_EXPECTED(close(epoll_fd));
 
   // All handles closed and all data read.
   result->set_stdout_data(out_data.GetData());
   result->set_stderr_data(err_data.GetData());
   DEBUG_ASSERT(out_data.IsEmpty());
   DEBUG_ASSERT(err_data.IsEmpty());
 
   // Calculate the exit code.
   intptr_t exit_code = exit_code_data.ints[0];
   intptr_t negative = exit_code_data.ints[1];
   if (negative != 0) {
     exit_code = -exit_code;
   }
   result->set_exit_code(exit_code);
 
   // Close the process handle.
   mx_handle_t process = static_cast<mx_handle_t>(pid);
   mx_handle_close(process);
   return true;
 }
 
-
 bool Process::Kill(intptr_t id, int signal) {
   LOG_INFO("Sending signal %d to process with id %ld\n", signal, id);
   // mx_task_kill is definitely going to kill the process.
   if ((signal != SIGTERM) && (signal != SIGKILL)) {
     LOG_ERR("Signal %d not supported\n", signal);
     errno = ENOSYS;
     return false;
   }
   // We can only use mx_task_kill if we know id is a process handle, and we only
   // know that for sure if it's in our list.
   mx_handle_t process = static_cast<mx_handle_t>(id);
   if (!ProcessInfoList::Exists(process)) {
     LOG_ERR("Process %ld wasn't in the ProcessInfoList\n", id);
     errno = ESRCH;  // No such process.
     return false;
   }
   mx_status_t status = mx_task_kill(process);
   if (status != NO_ERROR) {
     LOG_ERR("mx_task_kill failed: %s\n", mx_status_get_string(status));
     errno = EPERM;  // TODO(zra): Figure out what it really should be.
     return false;
   }
   LOG_INFO("Signal %d sent successfully to process %ld\n", signal, id);
   return true;
 }
 
-
 class ProcessStarter {
  public:
   ProcessStarter(const char* path,
                  char* arguments[],
                  intptr_t arguments_length,
                  const char* working_directory,
                  char* environment[],
                  intptr_t environment_length,
                  ProcessStartMode mode,
                  intptr_t* in,
@@ skipping 105 matching lines @@
     read_err_ = -1;
     FDUtils::SetNonBlocking(write_out_);
     *out_ = write_out_;
     write_out_ = -1;
     FDUtils::SetNonBlocking(exit_pipe_fds[0]);
     *exit_event_ = exit_pipe_fds[0];
     return 0;
   }
 
  private:
-#define CHECK_FOR_ERROR(status, msg)                                           \
-  if (status < 0) {                                                            \
-    const intptr_t kMaxMessageSize = 256;                                      \
-    char* message = DartUtils::ScopedCString(kMaxMessageSize);                 \
-    snprintf(message, kMaxMessageSize, "%s:%d: %s: %s\n", __FILE__, __LINE__,  \
-             msg, mx_status_get_string(status));                               \
-    *os_error_message_ = message;                                              \
-    return status;                                                             \
+#define CHECK_FOR_ERROR(status, msg)                                          \

[zra, 2017/05/26 16:13:22]

It looks like the escaped newlines were already in the right place.

+  if (status < 0) {                                                           \
+    const intptr_t kMaxMessageSize = 256;                                     \
+    char* message = DartUtils::ScopedCString(kMaxMessageSize);                \
+    snprintf(message, kMaxMessageSize, "%s:%d: %s: %s\n", __FILE__, __LINE__, \
+             msg, mx_status_get_string(status));                              \
+    *os_error_message_ = message;                                             \
+    return status;                                                            \
   }
 
   mx_status_t SetupLaunchpad(launchpad_t** launchpad) {
     // Set up a vmo for the binary.
     mx_handle_t binary_vmo = launchpad_vmo_from_file(path_);
     CHECK_FOR_ERROR(binary_vmo, "launchpad_vmo_from_file");
 
     // Run the child process in the same "job".
     mx_handle_t job = MX_HANDLE_INVALID;
     mx_status_t status =
@@ skipping 39 matching lines @@
   intptr_t* out_;
   intptr_t* err_;
   intptr_t* id_;
   intptr_t* exit_event_;
   char** os_error_message_;
 
   DISALLOW_ALLOCATION();
   DISALLOW_IMPLICIT_CONSTRUCTORS(ProcessStarter);
 };
 
-
 int Process::Start(const char* path,
                    char* arguments[],
                    intptr_t arguments_length,
                    const char* working_directory,
                    char* environment[],
                    intptr_t environment_length,
                    ProcessStartMode mode,
                    intptr_t* in,
                    intptr_t* out,
                    intptr_t* err,
                    intptr_t* id,
                    intptr_t* exit_event,
                    char** os_error_message) {
   if (mode != kNormal) {
     *os_error_message = DartUtils::ScopedCopyCString(
         "Only ProcessStartMode.NORMAL is supported on this platform");
     return -1;
   }
   ProcessStarter starter(path, arguments, arguments_length, working_directory,
                          environment, environment_length, mode, in, out, err,
                          id, exit_event, os_error_message);
   return starter.Start();
 }
 
-
 intptr_t Process::SetSignalHandler(intptr_t signal) {
   errno = ENOSYS;
   return -1;
 }
 
-
-void Process::ClearSignalHandler(intptr_t signal) {
-}
+void Process::ClearSignalHandler(intptr_t signal) {}
 
 }  // namespace bin
 }  // namespace dart
 
 #endif  // defined(HOST_OS_FUCHSIA)
 
 #endif  // !defined(DART_IO_DISABLED)