Merge pull request #18092 from apolcyn/backport_ares_windows_fix

Backport #18084 to 1.19.x

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_windows.cc

@@ -120,13 +120,14 @@ class GrpcPolledFdWindows : public GrpcPolledFd {
                     nullptr, &flags, (sockaddr*)recv_from_source_addr_,
                     &recv_from_source_addr_len_,
                     &winsocket_->read_info.overlapped, nullptr)) {
-      char* msg = gpr_format_message(WSAGetLastError());
+      int wsa_last_error = WSAGetLastError();
+      char* msg = gpr_format_message(wsa_last_error);
       grpc_error* error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
       GRPC_CARES_TRACE_LOG(
           "RegisterForOnReadableLocked: WSARecvFrom error:|%s|. fd:|%s|", msg,
           GetName());
       gpr_free(msg);
-      if (WSAGetLastError() != WSA_IO_PENDING) {
+      if (wsa_last_error != WSA_IO_PENDING) {
         ScheduleAndNullReadClosure(error);
         return;
       }
@@ -229,12 +230,13 @@ class GrpcPolledFdWindows : public GrpcPolledFd {
     ares_ssize_t total_sent;
     DWORD bytes_sent = 0;
     if (SendWriteBuf(&bytes_sent, nullptr) != 0) {
-      char* msg = gpr_format_message(WSAGetLastError());
+      int wsa_last_error = WSAGetLastError();
+      char* msg = gpr_format_message(wsa_last_error);
       GRPC_CARES_TRACE_LOG(
           "TrySendWriteBufSyncNonBlocking: SendWriteBuf error:|%s|. fd:|%s|",
           msg, GetName());
       gpr_free(msg);
-      if (WSAGetLastError() == WSA_IO_PENDING) {
+      if (wsa_last_error == WSA_IO_PENDING) {
         WSASetLastError(WSAEWOULDBLOCK);
         write_state_ = WRITE_REQUESTED;
       }
@@ -284,9 +286,10 @@ class GrpcPolledFdWindows : public GrpcPolledFd {
     int out =
         WSAConnect(s, target, target_len, nullptr, nullptr, nullptr, nullptr);
     if (out != 0) {
-      char* msg = gpr_format_message(WSAGetLastError());
+      int wsa_last_error = WSAGetLastError();
+      char* msg = gpr_format_message(wsa_last_error);
       GRPC_CARES_TRACE_LOG("Connect error code:|%d|, msg:|%s|. fd:|%s|",
-                           WSAGetLastError(), msg, GetName());
+                           wsa_last_error, msg, GetName());
       gpr_free(msg);
       // c-ares expects a posix-style connect API
       out = -1;

src/csharp/Grpc.IntegrationTesting/ExternalDnsClientServerTest.cs

@@ -0,0 +1,74 @@
+#region Copyright notice and license
+
+// Copyright 2019 The gRPC Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#endregion
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Grpc.Core;
+using Grpc.Core.Utils;
+using Grpc.Testing;
+using NUnit.Framework;
+
+namespace Grpc.IntegrationTesting
+{
+    /// <summary>
+    /// Runs interop tests in-process, with that client using a target
+    /// name that using a target name that triggers interaction with
+    /// external DNS servers (even though it resolves to the in-proc server).
+    /// This test is a trimmed-down sibling test to the one in
+    /// "ExternalDnsWithTracingClientServerTest", and is meant mostly for
+    /// comparison with that one.
+    /// </summary>
+    public class ExternalDnsClientServerTest
+    {
+        Server server;
+        Channel channel;
+        TestService.TestServiceClient client;
+
+        [OneTimeSetUp]
+        public void Init()
+        {
+            // Disable SO_REUSEPORT to prevent https://github.com/grpc/grpc/issues/10755
+            server = new Server(new[] { new ChannelOption(ChannelOptions.SoReuseport, 0) })
+            {
+                Services = { TestService.BindService(new TestServiceImpl()) },
+                Ports = { { "[::1]", ServerPort.PickUnused, ServerCredentials.Insecure } }
+            };
+            server.Start();
+
+            int port = server.Ports.Single().BoundPort;
+            channel = new Channel("loopback6.unittest.grpc.io", port, ChannelCredentials.Insecure);
+            client = new TestService.TestServiceClient(channel);
+        }
+
+        [OneTimeTearDown]
+        public void Cleanup()
+        {
+            channel.ShutdownAsync().Wait();
+            server.ShutdownAsync().Wait();
+        }
+
+        [Test]
+        public void EmptyUnary()
+        {
+            InteropClient.RunEmptyUnary(client);
+        }
+    }
+}

src/csharp/Grpc.IntegrationTesting/ExternalDnsWithTracingClientServerTest.cs

@@ -0,0 +1,167 @@
+#region Copyright notice and license
+
+// Copyright 2019 The gRPC Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#endregion
+
+using System;
+using System.Collections.Generic;
+using System.Net.Sockets;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Grpc.Core;
+using Grpc.Core.Logging;
+using Grpc.Core.Utils;
+using Grpc.Testing;
+using NUnit.Framework;
+
+namespace Grpc.IntegrationTesting
+{
+    /// <summary>
+    /// See https://github.com/grpc/issues/18074, this test is meant to
+    /// try to trigger the described bug.
+    /// Runs interop tests in-process, with that client using a target
+    /// name that using a target name that triggers interaction with
+    /// external DNS servers (even though it resolves to the in-proc server).
+    /// </summary>
+    public class ExternalDnsWithTracingClientServerTest
+    {
+        Server server;
+        Channel channel;
+        TestService.TestServiceClient client;
+        SocketUsingLogger newLogger;
+
+        [OneTimeSetUp]
+        public void Init()
+        {
+            // TODO(https://github.com/grpc/grpc/issues/14963): on linux, setting
+            // these environment variables might not actually have any affect.
+            // This is OK because we only really care about running this test on
+            // Windows, however, a fix made for $14963 should be applied here.
+            Environment.SetEnvironmentVariable("GRPC_TRACE", "all");
+            Environment.SetEnvironmentVariable("GRPC_VERBOSITY", "DEBUG");
+            newLogger = new SocketUsingLogger(GrpcEnvironment.Logger);
+            GrpcEnvironment.SetLogger(newLogger);
+            // Disable SO_REUSEPORT to prevent https://github.com/grpc/grpc/issues/10755
+            server = new Server(new[] { new ChannelOption(ChannelOptions.SoReuseport, 0) })
+            {
+                Services = { TestService.BindService(new TestServiceImpl()) },
+                Ports = { { "[::1]", ServerPort.PickUnused, ServerCredentials.Insecure } }
+            };
+            server.Start();
+
+            int port = server.Ports.Single().BoundPort;
+            channel = new Channel("loopback6.unittest.grpc.io", port, ChannelCredentials.Insecure);
+            client = new TestService.TestServiceClient(channel);
+        }
+
+        [OneTimeTearDown]
+        public void Cleanup()
+        {
+            channel.ShutdownAsync().Wait();
+            server.ShutdownAsync().Wait();
+        }
+
+        [Test]
+        public void EmptyUnary()
+        {
+            InteropClient.RunEmptyUnary(client);
+        }
+    }
+
+    /// <summary>
+    /// Logger which does some socket operation after delegating
+    /// actual logging to its delegate logger. The main goal is to
+    /// reset the current thread's WSA error status.
+    /// The only reason for the delegateLogger is to continue
+    /// to have this test display debug logs.
+    /// </summary>
+    internal sealed class SocketUsingLogger : ILogger
+    {
+        private ILogger delegateLogger;
+
+        public SocketUsingLogger(ILogger delegateLogger) {
+            this.delegateLogger = delegateLogger;
+        }
+
+        public void Debug(string message)
+        {
+            MyLog(() => delegateLogger.Debug(message));
+        }
+
+        public void Debug(string format, params object[] formatArgs)
+        {
+            MyLog(() => delegateLogger.Debug(format, formatArgs));
+        }
+
+        public void Error(string message)
+        {
+            MyLog(() => delegateLogger.Error(message));
+        }
+
+        public void Error(Exception exception, string message)
+        {
+            MyLog(() => delegateLogger.Error(exception, message));
+        }
+
+        public void Error(string format, params object[] formatArgs)
+        {
+            MyLog(() => delegateLogger.Error(format, formatArgs));
+        }
+
+        public ILogger ForType<T>()
+        {
+            return this;
+        }
+
+        public void Info(string message)
+        {
+            MyLog(() => delegateLogger.Info(message));
+        }
+
+        public void Info(string format, params object[] formatArgs)
+        {
+            MyLog(() => delegateLogger.Info(format, formatArgs));
+        }
+
+        public void Warning(string message)
+        {
+            MyLog(() => delegateLogger.Warning(message));
+        }
+
+        public void Warning(Exception exception, string message)
+        {
+            MyLog(() => delegateLogger.Warning(exception, message));
+        }
+
+        public void Warning(string format, params object[] formatArgs)
+        {
+            MyLog(() => delegateLogger.Warning(format, formatArgs));
+        }
+
+        private void MyLog(Action delegateLog)
+        {
+          delegateLog();
+          // Create and close a socket, just in order to affect
+          // the WSA (on Windows) error status of the current thread.
+          Socket s = new Socket(AddressFamily.InterNetwork,
+                                SocketType.Stream,
+                                ProtocolType.Tcp);
+
+          s.Dispose();
+        }
+    }
+}

src/csharp/tests.json

@@ -53,6 +53,8 @@
   ],
   "Grpc.IntegrationTesting": [
     "Grpc.IntegrationTesting.CustomErrorDetailsTest",
+    "Grpc.IntegrationTesting.ExternalDnsClientServerTest",
+    "Grpc.IntegrationTesting.ExternalDnsWithTracingClientServerTest",
     "Grpc.IntegrationTesting.GeneratedClientTest",
     "Grpc.IntegrationTesting.GeneratedServiceBaseTest",
     "Grpc.IntegrationTesting.HistogramTest",