Introduce a kSetEventMaskCommand, some cleanups in windows eventhandler

dart/runtime/bin/eventhandler_android.cc

 // Copyright (c) 2012, 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.
 
 #include "platform/globals.h"
 #if defined(TARGET_OS_ANDROID)
 
 #include "bin/eventhandler.h"
 
 #include <errno.h>  // NOLINT
@@ skipping 95 matching lines @@
                                             EPOLL_CTL_ADD,
                                             interrupt_fds_[0],
                                             &event));
   if (status == -1) {
     FATAL("Failed adding interrupt fd to epoll instance");
   }
 }
 
 
 EventHandlerImplementation::~EventHandlerImplementation() {
+  VOID_TEMP_FAILURE_RETRY(close(epoll_fd_));
   VOID_TEMP_FAILURE_RETRY(close(interrupt_fds_[0]));
   VOID_TEMP_FAILURE_RETRY(close(interrupt_fds_[1]));
 }
 
 
-SocketData* EventHandlerImplementation::GetSocketData(intptr_t fd) {
+SocketData* EventHandlerImplementation::GetSocketData(
+    intptr_t fd, bool listening_socket) {
   ASSERT(fd >= 0);
   HashMap::Entry* entry = socket_map_.Lookup(
       GetHashmapKeyFromFd(fd), GetHashmapHashFromFd(fd), true);
   ASSERT(entry != NULL);
   SocketData* sd = reinterpret_cast<SocketData*>(entry->value);
   if (sd == NULL) {
     // If there is no data in the hash map for this file descriptor a
     // new SocketData for the file descriptor is inserted.
-    sd = new SocketData(fd);
+    sd = new SocketData(fd, listening_socket);
     entry->value = sd;
   }
   ASSERT(fd == sd->fd());
   return sd;
 }
 
 
 void EventHandlerImplementation::WakeupHandler(intptr_t id,
                                                Dart_Port dart_port,
                                                int64_t data) {
@@ skipping 19 matching lines @@
 void EventHandlerImplementation::HandleInterruptFd() {
   const intptr_t MAX_MESSAGES = kInterruptMessageSize;
   InterruptMessage msg[MAX_MESSAGES];
   ssize_t bytes = TEMP_FAILURE_RETRY_NO_SIGNAL_BLOCKER(
       read(interrupt_fds_[0], msg, MAX_MESSAGES * kInterruptMessageSize));
   for (ssize_t i = 0; i < bytes / kInterruptMessageSize; i++) {
     if (msg[i].id == kTimerId) {
       timeout_queue_.UpdateTimeout(msg[i].dart_port, msg[i].data);
     } else if (msg[i].id == kShutdownId) {
       shutdown_ = true;
     } else {

[Søren Gjesse, 2015/02/06 08:09:25]

Maybe also add an assert that (msg[i].data & COMMAND_MASK != 0). It should be
caught by the UNREACHABLE further down, but would make it explicit that a
command bit it needed.

Maybe even better would be to asset that exactly one command bit is set.

[kustermann, 2015/02/06 09:36:47]

Done - although I find the UNREACHABLE enough.

The IS_COMMAND checks that it is exactly that command, so setting multiple
commands will fall through to UNREACHABLE.

-      SocketData* sd = GetSocketData(msg[i].id);
+      SocketData* sd = GetSocketData(
+          msg[i].id, (msg[i].data & (1 << kListeningSocket)) != 0);

[Søren Gjesse, 2015/02/06 08:09:25]

Maybe add LISTENING_MASK (defined as (1 << kListeningSocket))

[kustermann, 2015/02/06 09:36:47]

I defined a IS_LISTENING_SOCKET.

 
       if (IS_COMMAND(msg[i].data, kShutdownReadCommand)) {
         // Close the socket for reading.
         shutdown(sd->fd(), SHUT_RD);
       } else if (IS_COMMAND(msg[i].data, kShutdownWriteCommand)) {
         // Close the socket for writing.
         shutdown(sd->fd(), SHUT_WR);
       } else if (IS_COMMAND(msg[i].data, kCloseCommand)) {
         // Close the socket and free system resources and move on to
         // next message.
         RemoveFromEpollInstance(epoll_fd_, sd);
         intptr_t fd = sd->fd();
         sd->Close();
         socket_map_.Remove(GetHashmapKeyFromFd(fd), GetHashmapHashFromFd(fd));
         delete sd;
         DartUtils::PostInt32(msg[i].dart_port, 1 << kDestroyedEvent);
       } else if (IS_COMMAND(msg[i].data, kReturnTokenCommand)) {
         int count = TOKEN_COUNT(msg[i].data);
 
         for (int i = 0; i < count; i++) {
           if (sd->ReturnToken()) {
             AddToEpollInstance(epoll_fd_, sd);
           }
         }
+      } else if (IS_COMMAND(msg[i].data, kSetEventMaskCommand)) {
+        // `events` can only have kInEvent/kOutEvent flags set.
+        intptr_t events = msg[i].data & EVENT_MASK;
+        ASSERT(0 == (events & ~(1 << kInEvent) & ~(1 << kOutEvent)));

[Søren Gjesse, 2015/02/06 08:09:25]

I think

(events & ~(1 << kInEvent) | 1 << kOutEvent))

reads better

[kustermann, 2015/02/06 09:36:47]

Done.

+
+        // Setup events to wait for.
+        sd->SetPortAndMask(msg[i].dart_port, events);
+        AddToEpollInstance(epoll_fd_, sd);
       } else {
-        ASSERT_NO_COMMAND(msg[i].data);
-        // Setup events to wait for.
-        sd->SetPortAndMask(msg[i].dart_port, msg[i].data);
-        AddToEpollInstance(epoll_fd_, sd);
+        UNREACHABLE();
       }
     }
   }
 }
 
 #ifdef DEBUG_POLL
 static void PrintEventMask(intptr_t fd, intptr_t events) {
   Log::Print("%d ", fd);
   if ((events & EPOLLIN) != 0) Log::Print("EPOLLIN ");
   if ((events & EPOLLPRI) != 0) Log::Print("EPOLLPRI ");
@@ skipping 136 matching lines @@
 
 uint32_t EventHandlerImplementation::GetHashmapHashFromFd(intptr_t fd) {
   // The hashmap does not support keys with value 0.
   return dart::Utils::WordHash(fd + 1);
 }
 
 }  // namespace bin
 }  // namespace dart
 
 #endif  // defined(TARGET_OS_ANDROID)