Merge pull request #2733 from jtattermusch/csharp_backport_fixes

Backport C# related fixes to 0.10 branch.

Makefile

@@ -313,7 +313,7 @@ E = @echo
 Q = @
 endif
 
-VERSION = 0.10.0.0
+VERSION = 0.10.1.0
 
 CPPFLAGS_NO_ARCH += $(addprefix -I, $(INCLUDES)) $(addprefix -D, $(DEFINES))
 CPPFLAGS += $(CPPFLAGS_NO_ARCH) $(ARCH_FLAGS)

build.json

@@ -7,7 +7,7 @@
     "version": {
       "major": 0,
       "minor": 10,
-      "micro": 0,
+      "micro": 1,
       "build": 0
     }
   },

src/core/iomgr/iocp_windows.c

@@ -127,7 +127,6 @@ static void iocp_loop(void *p) {
     grpc_maybe_call_delayed_callbacks(NULL, 1);
     do_iocp_work();
   }
-  gpr_log(GPR_DEBUG, "iocp_loop is done");
 
   gpr_event_set(&g_iocp_done, (void *)1);
 }

src/core/iomgr/socket_windows.c

@@ -37,6 +37,7 @@
 
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
+#include <grpc/support/string_util.h>
 
 #include "src/core/iomgr/iocp_windows.h"
 #include "src/core/iomgr/iomgr_internal.h"
@@ -58,22 +59,27 @@ grpc_winsocket *grpc_winsocket_create(SOCKET socket, const char *name) {
    operations to abort them. We need to do that this way because of the
    various callsites of that function, which happens to be in various
    mutex hold states, and that'd be unsafe to call them directly. */
-int grpc_winsocket_shutdown(grpc_winsocket *socket) {
+int grpc_winsocket_shutdown(grpc_winsocket *winsocket) {
   int callbacks_set = 0;
-  gpr_mu_lock(&socket->state_mu);
-  if (socket->read_info.cb) {
+  SOCKET socket;
+  gpr_mu_lock(&winsocket->state_mu);
+  socket = winsocket->socket;
+  if (winsocket->read_info.cb) {
     callbacks_set++;
-    grpc_iomgr_closure_init(&socket->shutdown_closure, socket->read_info.cb,
-                            socket->read_info.opaque);
-    grpc_iomgr_add_delayed_callback(&socket->shutdown_closure, 0);
+    grpc_iomgr_closure_init(&winsocket->shutdown_closure,
+                            winsocket->read_info.cb,
+                            winsocket->read_info.opaque);
+    grpc_iomgr_add_delayed_callback(&winsocket->shutdown_closure, 0);
   }
-  if (socket->write_info.cb) {
+  if (winsocket->write_info.cb) {
     callbacks_set++;
-    grpc_iomgr_closure_init(&socket->shutdown_closure, socket->write_info.cb,
-                            socket->write_info.opaque);
-    grpc_iomgr_add_delayed_callback(&socket->shutdown_closure, 0);
+    grpc_iomgr_closure_init(&winsocket->shutdown_closure,
+                            winsocket->write_info.cb,
+                            winsocket->write_info.opaque);
+    grpc_iomgr_add_delayed_callback(&winsocket->shutdown_closure, 0);
   }
-  gpr_mu_unlock(&socket->state_mu);
+  gpr_mu_unlock(&winsocket->state_mu);
+  closesocket(socket);
   return callbacks_set;
 }
 
@@ -84,14 +90,12 @@ int grpc_winsocket_shutdown(grpc_winsocket *socket) {
    an "idle" socket which is neither trying to read or write, we'd start leaking
    both memory and sockets. */
 void grpc_winsocket_orphan(grpc_winsocket *winsocket) {
-  SOCKET socket = winsocket->socket;
   grpc_iomgr_unregister_object(&winsocket->iomgr_object);
   if (winsocket->read_info.outstanding || winsocket->write_info.outstanding) {
     grpc_iocp_socket_orphan(winsocket);
   } else {
     grpc_winsocket_destroy(winsocket);
   }
-  closesocket(socket);
 }
 
 void grpc_winsocket_destroy(grpc_winsocket *winsocket) {

src/core/iomgr/tcp_server_windows.c

@@ -79,6 +79,8 @@ struct grpc_tcp_server {
 
   /* active port count: how many ports are actually still listening */
   int active_ports;
+  /* number of iomgr callbacks that have been explicitly scheduled during shutdown */
+  int iomgr_callbacks_pending;
 
   /* all listening ports */
   server_port *ports;
@@ -93,6 +95,7 @@ grpc_tcp_server *grpc_tcp_server_create(void) {
   gpr_mu_init(&s->mu);
   gpr_cv_init(&s->cv);
   s->active_ports = 0;
+  s->iomgr_callbacks_pending = 0;
   s->cb = NULL;
   s->cb_arg = NULL;
   s->ports = gpr_malloc(sizeof(server_port) * INIT_PORT_CAP);
@@ -108,13 +111,14 @@ void grpc_tcp_server_destroy(grpc_tcp_server *s,
   size_t i;
   gpr_mu_lock(&s->mu);
   /* First, shutdown all fd's. This will queue abortion calls for all
-     of the pending accepts. */
+     of the pending accepts due to the normal operation mechanism. */
   for (i = 0; i < s->nports; i++) {
     server_port *sp = &s->ports[i];
-    grpc_winsocket_shutdown(sp->socket);
+    sp->shutting_down = 1;
+    s->iomgr_callbacks_pending += grpc_winsocket_shutdown(sp->socket);
   }
   /* This happens asynchronously. Wait while that happens. */
-  while (s->active_ports) {
+  while (s->active_ports || s->iomgr_callbacks_pending) {
     gpr_cv_wait(&s->cv, &s->mu, gpr_inf_future);
   }
   gpr_mu_unlock(&s->mu);
@@ -238,67 +242,65 @@ static void start_accept(server_port *port) {
 
 /* Event manager callback when reads are ready. */
 static void on_accept(void *arg, int from_iocp) {
+  DWORD transfered_bytes = 0;
+  DWORD flags;
+  BOOL wsa_success;
   server_port *sp = arg;
   SOCKET sock = sp->new_socket;
   grpc_winsocket_callback_info *info = &sp->socket->read_info;
   grpc_endpoint *ep = NULL;
 
-  /* The shutdown sequence is done in two parts. This is the second
-     part here, acknowledging the IOCP notification, and doing nothing
-     else, especially not queuing a new accept. */
-  if (sp->shutting_down) {
-    GPR_ASSERT(from_iocp);
-    sp->shutting_down = 0;
-    sp->socket->read_info.outstanding = 0;
+  /* The general mechanism for shutting down is to queue abortion calls. While
+     this is necessary in the read/write case, it's useless for the accept
+     case. We only need to adjust the pending callback count */
+  if (!from_iocp) {
     gpr_mu_lock(&sp->server->mu);
-    if (0 == --sp->server->active_ports) {
+    GPR_ASSERT(sp->server->iomgr_callbacks_pending > 0);
+    if (0 == --sp->server->iomgr_callbacks_pending) {
       gpr_cv_broadcast(&sp->server->cv);
     }
     gpr_mu_unlock(&sp->server->mu);
     return;
   }
 
-  if (from_iocp) {
-    /* The IOCP notified us of a completed operation. Let's grab the results,
-       and act accordingly. */
-    DWORD transfered_bytes = 0;
-    DWORD flags;
-    BOOL wsa_success = WSAGetOverlappedResult(sock, &info->overlapped,
-                                              &transfered_bytes, FALSE, &flags);
-    if (!wsa_success) {
+  /* The IOCP notified us of a completed operation. Let's grab the results,
+      and act accordingly. */
+  transfered_bytes = 0;
+  wsa_success = WSAGetOverlappedResult(sock, &info->overlapped,
+                                            &transfered_bytes, FALSE, &flags);
+  if (!wsa_success) {
+    if (sp->shutting_down) {
+      /* During the shutdown case, we ARE expecting an error. So that's well,
+         and we can wake up the shutdown thread. */
+      sp->shutting_down = 0;
+      sp->socket->read_info.outstanding = 0;
+      gpr_mu_lock(&sp->server->mu);
+      GPR_ASSERT(sp->server->active_ports > 0);
+      if (0 == --sp->server->active_ports) {
+        gpr_cv_broadcast(&sp->server->cv);
+      }
+      gpr_mu_unlock(&sp->server->mu);
+      return;
+    } else {
       char *utf8_message = gpr_format_message(WSAGetLastError());
       gpr_log(GPR_ERROR, "on_accept error: %s", utf8_message);
       gpr_free(utf8_message);
       closesocket(sock);
-    } else {
-	  /* TODO(ctiller): add sockaddr address to label */
-      ep = grpc_tcp_create(grpc_winsocket_create(sock, "server"));
     }
   } else {
-    /* If we're not notified from the IOCP, it means we are asked to shutdown.
-       This will initiate that shutdown. Calling closesocket will trigger an
-       IOCP notification, that will call this function a second time, from
-       the IOCP thread. Of course, this only works if the socket was, in fact,
-       listening. If that's not the case, we'd wait indefinitely. That's a bit
-       of a degenerate case, but it can happen if you create a server, but
-       don't start it. So let's support that by recursing once. */
-    sp->shutting_down = 1;
-    sp->new_socket = INVALID_SOCKET;
-    if (sock != INVALID_SOCKET) {
-      closesocket(sock);
-    } else {
-      on_accept(sp, 1);
+    if (!sp->shutting_down) {
+      /* TODO(ctiller): add sockaddr address to label */
+      ep = grpc_tcp_create(grpc_winsocket_create(sock, "server"));
     }
-    return;
   }
 
   /* The only time we should call our callback, is where we successfully
      managed to accept a connection, and created an endpoint. */
   if (ep) sp->server->cb(sp->server->cb_arg, ep);
   /* As we were notified from the IOCP of one and exactly one accept,
-      the former socked we created has now either been destroy or assigned
-      to the new connection. We need to create a new one for the next
-      connection. */
+     the former socked we created has now either been destroy or assigned
+     to the new connection. We need to create a new one for the next
+     connection. */
   start_accept(sp);
 }
 

src/core/iomgr/tcp_windows.c

@@ -148,9 +148,11 @@ static void on_read(void *tcpp, int from_iocp) {
   GPR_ASSERT(tcp->socket->read_info.outstanding);
 
   if (socket->read_info.wsa_error != 0) {
-    char *utf8_message = gpr_format_message(info->wsa_error);
-    gpr_log(GPR_ERROR, "ReadFile overlapped error: %s", utf8_message);
-    gpr_free(utf8_message);
+    if (socket->read_info.wsa_error != WSAECONNRESET) {
+      char *utf8_message = gpr_format_message(info->wsa_error);
+      gpr_log(GPR_ERROR, "ReadFile overlapped error: %s", utf8_message);
+      gpr_free(utf8_message);
+    }
     status = GRPC_ENDPOINT_CB_ERROR;
   } else {
     if (info->bytes_transfered != 0) {
@@ -259,9 +261,11 @@ static void on_write(void *tcpp, int from_iocp) {
   GPR_ASSERT(tcp->socket->write_info.outstanding);
 
   if (info->wsa_error != 0) {
-    char *utf8_message = gpr_format_message(info->wsa_error);
-    gpr_log(GPR_ERROR, "WSASend overlapped error: %s", utf8_message);
-    gpr_free(utf8_message);
+    if (info->wsa_error != WSAECONNRESET) {
+      char *utf8_message = gpr_format_message(info->wsa_error);
+      gpr_log(GPR_ERROR, "WSASend overlapped error: %s", utf8_message);
+      gpr_free(utf8_message);
+    }
     status = GRPC_ENDPOINT_CB_ERROR;
   } else {
     GPR_ASSERT(info->bytes_transfered == tcp->write_slices.length);
@@ -325,9 +329,11 @@ static grpc_endpoint_write_status win_write(grpc_endpoint *ep,
       ret = GRPC_ENDPOINT_WRITE_DONE;
       GPR_ASSERT(bytes_sent == tcp->write_slices.length);
     } else {
-      char *utf8_message = gpr_format_message(info->wsa_error);
-      gpr_log(GPR_ERROR, "WSASend error: %s", utf8_message);
-      gpr_free(utf8_message);
+      if (socket->read_info.wsa_error != WSAECONNRESET) {
+        char *utf8_message = gpr_format_message(info->wsa_error);
+        gpr_log(GPR_ERROR, "WSASend error: %s", utf8_message);
+        gpr_free(utf8_message);
+      }
     }
     if (allocated) gpr_free(allocated);
     gpr_slice_buffer_reset_and_unref(&tcp->write_slices);

src/core/surface/channel.c

@@ -91,6 +91,7 @@ grpc_channel *grpc_channel_create_from_filters(
   size_t size =
       sizeof(grpc_channel) + grpc_channel_stack_size(filters, num_filters);
   grpc_channel *channel = gpr_malloc(size);
+  memset(channel, 0, sizeof(*channel));
   GPR_ASSERT(grpc_is_initialized() && "call grpc_init()");
   channel->is_client = is_client;
   /* decremented by grpc_channel_destroy */

src/csharp/Grpc.Auth/Grpc.Auth.csproj

@@ -68,6 +68,9 @@
     <Reference Include="System.Net.Http.WebRequest" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="..\Grpc.Core\Version.cs">
+      <Link>Version.cs</Link>
+    </Compile>
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="GoogleCredential.cs" />
     <Compile Include="OAuth2InterceptorFactory.cs" />
@@ -81,6 +84,7 @@
   </ItemGroup>
   <ItemGroup>
     <None Include="app.config" />
+    <None Include="Grpc.Auth.nuspec" />
     <None Include="packages.config" />
   </ItemGroup>
   <Import Project="..\packages\Microsoft.Bcl.Build.1.0.14\tools\Microsoft.Bcl.Build.targets" Condition="Exists('..\packages\Microsoft.Bcl.Build.1.0.14\tools\Microsoft.Bcl.Build.targets')" />

src/csharp/Grpc.Auth/Grpc.Auth.nuspec

@@ -5,19 +5,19 @@
     <title>gRPC C# Auth</title>
     <summary>Auth library for C# implementation of gRPC - an RPC library and framework</summary>
     <description>Auth library for C# implementation of gRPC - an RPC library and framework. See project site for more info.</description>
-    <version>0.6.0</version>
+    <version>$version$</version>
     <authors>Google Inc.</authors>
     <owners>grpc-packages</owners>
     <licenseUrl>https://github.com/grpc/grpc/blob/master/LICENSE</licenseUrl>
     <projectUrl>https://github.com/grpc/grpc</projectUrl>
     <requireLicenseAcceptance>false</requireLicenseAcceptance>
-    <releaseNotes>Release 0.6.0 of gRPC C#</releaseNotes>
+    <releaseNotes>Release $version$ of gRPC C#</releaseNotes>
     <copyright>Copyright 2015, Google Inc.</copyright>
     <tags>gRPC RPC Protocol HTTP/2 Auth OAuth2</tags>
 	<dependencies>
 	  <dependency id="BouncyCastle" version="1.7.0" />
 	  <dependency id="Google.Apis.Auth" version="1.9.1" />
-	  <dependency id="Grpc.Core" version="0.6.0" />
+	  <dependency id="Grpc.Core" version="$version$" />
     </dependencies>
   </metadata>
   <files>

src/csharp/Grpc.Auth/Properties/AssemblyInfo.cs

@@ -9,6 +9,5 @@
 [assembly: AssemblyCopyright("Google Inc.  All rights reserved.")]
 [assembly: AssemblyTrademark("")]
 [assembly: AssemblyCulture("")]
-[assembly: AssemblyVersion("0.6.*")]
 
 [assembly: InternalsVisibleTo("Grpc.Auth.Tests")]

src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj

@@ -37,6 +37,9 @@
     </Reference>
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="..\Grpc.Core\Version.cs">
+      <Link>Version.cs</Link>
+    </Compile>
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="ClientServerTest.cs" />
     <Compile Include="ServerTest.cs" />
@@ -63,4 +66,4 @@
     <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
   </ItemGroup>
   <ItemGroup />
-</Project>
+</Project>

src/csharp/Grpc.Core.Tests/Properties/AssemblyInfo.cs

@@ -9,4 +9,3 @@
 [assembly: AssemblyCopyright("Google Inc.  All rights reserved.")]
 [assembly: AssemblyTrademark("")]
 [assembly: AssemblyCulture("")]
-[assembly: AssemblyVersion("0.6.*")]

src/csharp/Grpc.Core/Grpc.Core.csproj

@@ -49,6 +49,7 @@
     <Compile Include="IAsyncStreamWriter.cs" />
     <Compile Include="IAsyncStreamReader.cs" />
     <Compile Include="Internal\GrpcLog.cs" />
+    <Compile Include="Version.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="RpcException.cs" />
     <Compile Include="Calls.cs" />
@@ -104,6 +105,7 @@
     <Compile Include="ChannelOptions.cs" />
   </ItemGroup>
   <ItemGroup>
+    <None Include="Grpc.Core.nuspec" />
     <None Include="packages.config" />
   </ItemGroup>
   <Choose>

src/csharp/Grpc.Core/Grpc.Core.nuspec

@@ -5,19 +5,19 @@
     <title>gRPC C# Core</title>
     <summary>Core C# implementation of gRPC - an RPC library and framework</summary>
     <description>Core C# implementation of gRPC - an RPC library and framework. See project site for more info.</description>
-    <version>0.6.0</version>
+    <version>$version$</version>
     <authors>Google Inc.</authors>
     <owners>grpc-packages</owners>
     <licenseUrl>https://github.com/grpc/grpc/blob/master/LICENSE</licenseUrl>
     <projectUrl>https://github.com/grpc/grpc</projectUrl>
     <requireLicenseAcceptance>false</requireLicenseAcceptance>
-    <releaseNotes>Release 0.6.0 of gRPC C#</releaseNotes>
+    <releaseNotes>Release $version$ of gRPC C#</releaseNotes>
     <copyright>Copyright 2015, Google Inc.</copyright>
     <tags>gRPC RPC Protocol HTTP/2</tags>
 	<dependencies>
 	  <dependency id="System.Collections.Immutable" version="1.1.36" />
 	  <dependency id="Ix-Async" version="1.2.3" />
-	  <dependency id="grpc.native.csharp_ext" version="0.10.0" />
+	  <dependency id="grpc.native.csharp_ext" version="$GrpcNativeCsharpExtVersion$" />
     </dependencies>
   </metadata>
   <files>

src/csharp/Grpc.Core/Properties/AssemblyInfo.cs

@@ -9,6 +9,5 @@
 [assembly: AssemblyCopyright("Google Inc.  All rights reserved.")]
 [assembly: AssemblyTrademark("")]
 [assembly: AssemblyCulture("")]
-[assembly: AssemblyVersion("0.6.*")]
 
 [assembly: InternalsVisibleTo("Grpc.Core.Tests")]