Introduce a kSetEventMaskCommand, some cleanups in windows eventhandler

dart/runtime/bin/eventhandler_android.cc

Index: dart/runtime/bin/eventhandler_android.cc
diff --git a/dart/runtime/bin/eventhandler_android.cc b/dart/runtime/bin/eventhandler_android.cc
index dbe26ccd1f28d650888982696e533d9de9637f4f..6963ee2bb37bf4bc3f8669cb3282342ccc9c9974 100644
--- a/dart/runtime/bin/eventhandler_android.cc
+++ b/dart/runtime/bin/eventhandler_android.cc
@@ -113,12 +113,14 @@ EventHandlerImplementation::EventHandlerImplementation()
 
 
 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);
@@ -127,7 +129,7 @@ SocketData* EventHandlerImplementation::GetSocketData(intptr_t fd) {
   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());
@@ -168,7 +170,8 @@ void EventHandlerImplementation::HandleInterruptFd() {
     } 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.
@@ -193,11 +196,16 @@ void EventHandlerImplementation::HandleInterruptFd() {
             AddToEpollInstance(epoll_fd_, sd);
           }
         }
-      } else {
-        ASSERT_NO_COMMAND(msg[i].data);
+      } 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, msg[i].data);
+        sd->SetPortAndMask(msg[i].dart_port, events);
         AddToEpollInstance(epoll_fd_, sd);
+      } else {
+        UNREACHABLE();
       }
     }
   }