From 82d1382faa36d1b6f731b341de5f29042fc4af4b Mon Sep 17 00:00:00 2001
From: "ajohnsen@google.com" <ajohnsen@google.com>
Date: Mon, 5 May 2014 09:17:26 +0000
Subject: [PATCH] Fix crash in raw_secure_server_socket_test.

We had a scenario where read&write events were requested after the socket was closed, leading to a use-after-free.

This also fixes a scenario where we didn't delete client sockets, when accepted sockets are ignored when closing a listening socket.

BUG=https://code.google.com/p/dart/issues/detail?id=18610
R=sgjesse@google.com

Review URL: https://codereview.chromium.org//269003002

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@35732 260f80e4-7a28-3924-810f-c04153c831b5
---
 runtime/bin/eventhandler_win.cc    | 27 ++++++++++++++++-----------
 runtime/bin/eventhandler_win.h     |  2 +-
 runtime/bin/socket_patch.dart      |  4 +++-
 tests/standalone/standalone.status |  3 ---
 4 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/runtime/bin/eventhandler_win.cc b/runtime/bin/eventhandler_win.cc
index 754a5b96d6b..7e1a2b33d96 100644
--- a/runtime/bin/eventhandler_win.cc
+++ b/runtime/bin/eventhandler_win.cc
@@ -334,6 +334,7 @@ bool Handle::IssueSendTo(struct sockaddr* sa, socklen_t sa_len) {
 
 void Handle::HandleIssueError() {
   DWORD error = GetLastError();
+  ASSERT(event_handler_ != NULL);
   if (error == ERROR_BROKEN_PIPE) {
     event_handler_->HandleClosed(this);
   } else {
@@ -399,6 +400,7 @@ bool DirectoryWatchHandle::IssueRead() {
 
 void SocketHandle::HandleIssueError() {
   int error = WSAGetLastError();
+  ASSERT(event_handler_ != NULL);
   if (error == WSAECONNRESET) {
     event_handler_->HandleClosed(this);
   } else {
@@ -502,6 +504,17 @@ void ListenSocket::AcceptComplete(OverlappedBuffer* buffer,
 }
 
 
+static void DeleteIfClosed(Handle* handle) {
+  if (handle->IsClosed()) {
+    Dart_Port port = handle->port();
+    delete handle;
+    if (port != ILLEGAL_PORT) {
+      DartUtils::PostInt32(port, 1 << kDestroyedEvent);
+    }
+  }
+}
+
+
 void ListenSocket::DoClose() {
   closesocket(socket());
   handle_ = INVALID_HANDLE_VALUE;
@@ -510,6 +523,7 @@ void ListenSocket::DoClose() {
     ClientSocket *client = Accept();
     if (client != NULL) {
       client->Close();
+      DeleteIfClosed(client);
     } else {
       break;
     }
@@ -955,18 +969,8 @@ void DatagramSocket::DoClose() {
 }
 
 
-static void DeleteIfClosed(Handle* handle) {
-  if (handle->IsClosed()) {
-    Dart_Port port = handle->port();
-    delete handle;
-    if (port != ILLEGAL_PORT) {
-      DartUtils::PostInt32(port, 1 << kDestroyedEvent);
-    }
-  }
-}
-
-
 void EventHandlerImplementation::HandleInterrupt(InterruptMessage* msg) {
+  ASSERT(this != NULL);
   if (msg->id == kTimeoutId) {
     // Change of timeout request. Just set the new timeout and port as the
     // completion thread will use the new timeout value for its next wait.
@@ -1289,6 +1293,7 @@ void EventHandlerImplementation::EventHandlerEntry(uword args) {
                                         &key,
                                         &overlapped,
                                         static_cast<DWORD>(millis));
+
     if (!ok && overlapped == NULL) {
       if (GetLastError() == ERROR_ABANDONED_WAIT_0) {
         // The completion port should never be closed.
diff --git a/runtime/bin/eventhandler_win.h b/runtime/bin/eventhandler_win.h
index 89b194131d0..ef4157af4ae 100644
--- a/runtime/bin/eventhandler_win.h
+++ b/runtime/bin/eventhandler_win.h
@@ -224,7 +224,6 @@ class Handle {
 
   HANDLE handle() { return handle_; }
   Dart_Port port() { return port_; }
-  EventHandlerImplementation* event_handler() { return event_handler_; }
 
   void Lock();
   void Unlock();
@@ -439,6 +438,7 @@ class ClientSocket : public SocketHandle {
     ASSERT(!HasPendingRead());
     ASSERT(!HasPendingWrite());
     ASSERT(next_ == NULL);
+    ASSERT(closed_ == true);
   };
 
   void Shutdown(int how);
diff --git a/runtime/bin/socket_patch.dart b/runtime/bin/socket_patch.dart
index 030a9058cc9..98de2a0bea0 100644
--- a/runtime/bin/socket_patch.dart
+++ b/runtime/bin/socket_patch.dart
@@ -824,6 +824,7 @@ class _NativeSocket extends NativeFieldWrapperClass1 {
 
         var handler = eventHandlers[i];
         if (i == DESTROYED_EVENT) {
+          assert(isClosing);
           assert(!isClosed);
           isClosed = true;
           closeCompleter.complete();
@@ -873,7 +874,7 @@ class _NativeSocket extends NativeFieldWrapperClass1 {
     sendWriteEvents = write;
     if (read) issueReadEvent();
     if (write) issueWriteEvent();
-    if (eventPort == null) {
+    if (eventPort == null && !isClosing) {
       int flags = typeFlags & TYPE_TYPE_MASK;
       if (!isClosedRead) flags |= 1 << READ_EVENT;
       if (!isClosedWrite) flags |= 1 << WRITE_EVENT;
@@ -936,6 +937,7 @@ class _NativeSocket extends NativeFieldWrapperClass1 {
   }
 
   void connectToEventHandler() {
+    assert(!isClosed);
     if (eventPort == null) {
       eventPort = new RawReceivePort(multiplex);
       _SocketsObservatory.add(this);
diff --git a/tests/standalone/standalone.status b/tests/standalone/standalone.status
index f56be422845..62de2f0c318 100644
--- a/tests/standalone/standalone.status
+++ b/tests/standalone/standalone.status
@@ -11,9 +11,6 @@ package/invalid_uri_test: Fail, OK # CompileTimeErrors intentionally
 
 issue14236_test: Pass # Do not remove this line. It serves as a marker for Issue 14516 comment #4.
 
-[ $runtime == vm && $system == windows ]
-io/raw_secure_server_socket_test: Pass, Crash # Issue 18610
-
 [ $runtime == vm ]
 package/package_isolate_test: Fail # Issue 12474