Added synchronous socket implementation to dart:io.

runtime/bin/sync_socket.cc

Index: runtime/bin/sync_socket.cc
diff --git a/runtime/bin/sync_socket.cc b/runtime/bin/sync_socket.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6764957a6da336c3e62bcb02874f281b6cbea165
--- /dev/null
+++ b/runtime/bin/sync_socket.cc
@@ -0,0 +1,276 @@
+// Copyright (c) 2017, 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 "bin/sync_socket.h"
+
+#include "bin/dartutils.h"
+#include "bin/io_buffer.h"
+#include "bin/isolate_data.h"
+#include "bin/lockers.h"
+#include "bin/thread.h"
+#include "bin/utils.h"
+
+#include "include/dart_api.h"
+
+#include "platform/globals.h"
+#include "platform/utils.h"
+
+namespace dart {
+namespace bin {
+
+static const int kSocketIdNativeField = 0;
+
+
+void FUNCTION_NAME(SynchronousSocket_LookupRequest)(Dart_NativeArguments args) {
+  if (Dart_GetNativeArgumentCount(args) == 2) {

[zra, 2017/04/07 05:29:27]

The logic can be rearranged a bit to avoid a lot of indentation. E.g.:

if (DartGetNativeArgumentCount(args) != 2) {
  // set error
  return;
}
...
if (addresses == NULL) {
  // set error
  return;
}

etc.

[bkonyi, 2017/04/10 19:20:05]

Done.

+    const char* host =
+        DartUtils::GetStringValue(Dart_GetNativeArgument(args, 0));

[zra, 2017/04/07 05:29:27]

See Dart_GetNativeStringArgument instead

[bkonyi, 2017/04/10 19:20:04]

Done.

+    int type = DartUtils::GetIntegerValue(Dart_GetNativeArgument(args, 1));

[zra, 2017/04/07 05:29:27]

See Dart_GetNativeIntegerArgument instead

[bkonyi, 2017/04/10 19:20:04]

Done.

+
+    Dart_Handle result = NULL;
+    OSError* os_error = NULL;
+    AddressList<SocketAddress>* addresses =
+        SocketBase::LookupAddress(host, type, &os_error);
+    if (addresses != NULL) {
+      Dart_Handle array = Dart_NewList(addresses->count() + 1);

[zra, 2017/04/07 05:29:27]

If Dart API calls can return error handles, then we need to check for them with
Dart_IsError() and Dart_PropagateError() if so. We are probably not as
consistent about this everywhere in runtime/bin as we should be, but we can do
it right here.

[bkonyi, 2017/04/10 19:20:04]

Done.

+      Dart_ListSetAt(array, 0, Dart_NewInteger(0));

[zra, 2017/04/07 05:29:27]

Why is this here?

[bkonyi, 2017/04/10 19:20:04]

There's a TODO in sync_socket_patch which references this. The async socket
implementation did this, my guess is to allow for an error code to be returned,
but I couldn't find out where it was set.

+      for (intptr_t i = 0; i < addresses->count(); i++) {
+        SocketAddress* addr = addresses->GetAt(i);
+        Dart_Handle entry = Dart_NewList(3);
+
+        Dart_Handle type = Dart_NewInteger(addr->GetType());
+        Dart_ListSetAt(entry, 0, type);
+
+        Dart_Handle as_string = Dart_NewStringFromCString(addr->as_string());
+        Dart_ListSetAt(entry, 1, as_string);
+
+        RawAddr raw = addr->addr();
+        Dart_Handle data = SocketAddress::ToTypedData(raw);
+        Dart_ListSetAt(entry, 2, data);
+
+        Dart_ListSetAt(array, i + 1, entry);
+      }
+      result = array;
+      delete addresses;
+    } else {
+      result = DartUtils::NewDartOSError(os_error);
+      delete os_error;
+    }
+    Dart_SetReturnValue(args, result);
+    return;
+  }
+  Dart_SetReturnValue(args, DartUtils::NewDartArgumentError(""));

[zra, 2017/04/07 05:29:27]

Can you give an error message here?

[bkonyi, 2017/04/10 19:20:04]

Done.

+}
+
+
+void FUNCTION_NAME(SynchronousSocket_CreateConnectSync)(
+    Dart_NativeArguments args) {
+  RawAddr addr;
+  SocketAddress::GetSockAddr(Dart_GetNativeArgument(args, 1), &addr);
+  Dart_Handle port_arg = Dart_GetNativeArgument(args, 2);
+  int64_t port = DartUtils::GetInt64ValueCheckRange(port_arg, 0, 65535);
+  SocketAddress::SetAddrPort(&addr, static_cast<intptr_t>(port));
+
+  intptr_t socket = SynchronousSocket::CreateConnect(addr);
+  OSError error;

[zra, 2017/04/07 05:29:27]

This declaration should go inside the else case. the OSError constructor can
allocate memory and might do a bunch of other stuff depending on the platform,
so we should only do that work in the error case.

[bkonyi, 2017/04/10 19:20:05]

Done.

+  if (socket >= 0) {
+    SynchronousSocket::SetSocketIdNativeField(Dart_GetNativeArgument(args, 0),
+                                              socket);
+    Dart_SetReturnValue(args, Dart_True());
+  } else {
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError(&error));
+  }
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_WriteList)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  Dart_Handle buffer_obj = Dart_GetNativeArgument(args, 1);
+  ASSERT(Dart_IsList(buffer_obj));

[zra, 2017/04/07 05:29:27]

This is probably an assert because the argument comes from the patch file, but I
think it would be better to be consistent about how we're checking arguments,
and instead return an ArgumentError.

[bkonyi, 2017/04/10 19:20:05]

Done.

+  intptr_t offset = DartUtils::GetIntptrValue(Dart_GetNativeArgument(args, 2));
+  intptr_t length = DartUtils::GetIntptrValue(Dart_GetNativeArgument(args, 3));
+  Dart_TypedData_Type type;
+  uint8_t* buffer = NULL;
+  intptr_t len;
+  Dart_Handle result = Dart_TypedDataAcquireData(
+      buffer_obj, &type, reinterpret_cast<void**>(&buffer), &len);
+  if (Dart_IsError(result)) {
+    Dart_PropagateError(result);
+  }
+  ASSERT((offset + length) <= len);
+  buffer += offset;
+  intptr_t bytes_written = SocketBase::Write(socket->fd(), buffer, length);
+  if (bytes_written >= 0) {
+    Dart_TypedDataReleaseData(buffer_obj);
+    Dart_SetReturnValue(args, Dart_NewInteger(bytes_written));
+  } else {
+    // Extract OSError before we release data, as it may override the error.
+    OSError os_error;
+    Dart_TypedDataReleaseData(buffer_obj);
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError(&os_error));
+  }
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_Available)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  intptr_t available = SocketBase::Available(socket->fd());
+  if (available >= 0) {
+    Dart_SetReturnValue(args, Dart_NewInteger(available));
+  } else {
+    // Available failed. Mark socket as having data, to trigger a future read

[zra, 2017/04/07 05:29:27]

This doesn't make any sense for a synchronous socket. Instead, we should return
an OSError here.

[bkonyi, 2017/04/10 19:20:04]

Done.

+    // event where the actual error can be reported.
+    Dart_SetReturnValue(args, Dart_NewInteger(1));
+  }
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_CloseSync)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  SocketBase::Close(socket->fd());
+  socket->SetClosedFd();
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_Read)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  int64_t length = 0;
+  if (DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 1), &length)) {
+    uint8_t* buffer = NULL;
+    Dart_Handle result = IOBuffer::Allocate(length, &buffer);
+    if (Dart_IsError(result)) {
+      Dart_PropagateError(result);
+    }
+    ASSERT(buffer != NULL);
+    intptr_t bytes_read = SocketBase::Read(socket->fd(), buffer, length);
+    if (bytes_read == length) {
+      Dart_SetReturnValue(args, result);
+    } else if (bytes_read > 0) {
+      uint8_t* new_buffer = NULL;
+      Dart_Handle new_result = IOBuffer::Allocate(bytes_read, &new_buffer);
+      if (Dart_IsError(new_result)) {
+        Dart_PropagateError(new_result);
+      }
+      ASSERT(new_buffer != NULL);
+      memmove(new_buffer, buffer, bytes_read);
+      Dart_SetReturnValue(args, new_result);
+    } else if (bytes_read == 0) {
+      Dart_SetReturnValue(args, Dart_Null());
+    } else {
+      ASSERT(bytes_read == -1);
+      Dart_SetReturnValue(args, DartUtils::NewDartOSError());
+    }
+  } else {
+    OSError os_error(-1, "Invalid argument", OSError::kUnknown);
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError(&os_error));

[zra, 2017/04/07 16:29:40]

Above, you return an ArgumentError on a bad argument. Why is it an OSError here?

[bkonyi, 2017/04/10 19:20:05]

Not sure... Probably something that migrated from the socket.cc implementation
that I didn't catch. Changed.

+  }
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_ShutdownRead)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  SynchronousSocket::ShutdownRead(socket->fd());
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_ShutdownWrite)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  SynchronousSocket::ShutdownWrite(socket->fd());
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_GetPort)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  OSError os_error;

[zra, 2017/04/07 05:29:28]

Not needed since you are using DartUtils::NewDartOSError() below.

[bkonyi, 2017/04/10 19:20:04]

Done.

+  intptr_t port = SocketBase::GetPort(socket->fd());
+  if (port > 0) {
+    Dart_SetReturnValue(args, Dart_NewInteger(port));
+  } else {
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError());
+  }
+}
+
+
+void FUNCTION_NAME(SynchronousSocket_GetRemotePeer)(Dart_NativeArguments args) {
+  SynchronousSocket* socket = SynchronousSocket::GetSocketIdNativeField(
+      Dart_GetNativeArgument(args, 0));
+  OSError os_error;

[zra, 2017/04/07 05:29:27]

ditto

[bkonyi, 2017/04/10 19:20:05]

Done.

+  intptr_t port = 0;
+  SocketAddress* addr = SocketBase::GetRemotePeer(socket->fd(), &port);
+  if (addr != NULL) {

[zra, 2017/04/07 05:29:27]

if (addr == NULL) {
  // set error
  return;
}

[bkonyi, 2017/04/10 19:20:04]

Done.

+    Dart_Handle list = Dart_NewList(2);
+
+    Dart_Handle entry = Dart_NewList(3);
+    Dart_ListSetAt(entry, 0, Dart_NewInteger(addr->GetType()));
+    Dart_ListSetAt(entry, 1, Dart_NewStringFromCString(addr->as_string()));
+
+    RawAddr raw = addr->addr();
+    Dart_ListSetAt(entry, 2, SocketAddress::ToTypedData(raw));
+
+    Dart_ListSetAt(list, 0, entry);
+    Dart_ListSetAt(list, 1, Dart_NewInteger(port));
+    Dart_SetReturnValue(args, list);
+    delete addr;
+  } else {
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError());
+  }
+}
+
+
+static void SynchronousSocketFinalizer(void* isolate_data,
+                                       Dart_WeakPersistentHandle handle,
+                                       void* data) {
+  SynchronousSocket* socket = reinterpret_cast<SynchronousSocket*>(data);
+  if (socket->fd() >= 0) {
+    SocketBase::Close(socket->fd());
+    socket->SetClosedFd();
+  }

[zra, 2017/04/07 05:29:28]

delete socket;

[bkonyi, 2017/04/10 19:20:04]

Done.

+}
+
+
+void SynchronousSocket::ReuseSocketIdNativeField(Dart_Handle handle,

[zra, 2017/04/07 05:29:27]

Not needed.

[bkonyi, 2017/04/10 19:20:04]

Removed.

+                                                 SynchronousSocket* socket) {
+  Dart_Handle err = Dart_SetNativeInstanceField(
+      handle, kSocketIdNativeField, reinterpret_cast<intptr_t>(socket));
+  if (Dart_IsError(err)) {
+    Dart_PropagateError(err);
+  }
+  Dart_NewWeakPersistentHandle(handle, reinterpret_cast<void*>(socket),
+                               sizeof(SynchronousSocket),
+                               SynchronousSocketFinalizer);
+}
+
+
+void SynchronousSocket::SetSocketIdNativeField(Dart_Handle handle,
+                                               intptr_t id) {
+  SynchronousSocket* socket = new SynchronousSocket(id);
+  ReuseSocketIdNativeField(handle, socket);
+}
+
+
+SynchronousSocket* SynchronousSocket::GetSocketIdNativeField(
+    Dart_Handle socket_obj) {
+  intptr_t id;
+  Dart_Handle err =
+      Dart_GetNativeInstanceField(socket_obj, kSocketIdNativeField, &id);
+  if (Dart_IsError(err)) {
+    Dart_PropagateError(err);
+  }
+  SynchronousSocket* socket = reinterpret_cast<SynchronousSocket*>(id);
+  return socket;
+}
+
+}  // namespace bin
+}  // namespace dart
+
+#endif  // !defined(DART_IO_DISABLED)