From 31af304f092df7bdb5e036451d863ba974bebca3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20K=C3=B6llner?= <christian.koellner.80@gmail.com>
Date: Sun, 7 Jul 2019 19:59:31 +0200
Subject: [PATCH] See comment on issue #3: TcpRpcClient+TcpRpcServer should
 retry if socket cannot be bound due to SocketError.AddressAlreadyInUse

---
 Capnp.Net.Runtime.Tests/TcpRpc.cs             | 38 ++++++++++++-------
 .../TcpRpcAdvancedStuff.cs                    |  4 +-
 Capnp.Net.Runtime.Tests/TcpRpcInterop.cs      | 26 ++++++-------
 Capnp.Net.Runtime.Tests/TcpRpcPorted.cs       | 28 +++++++-------
 Capnp.Net.Runtime.Tests/TcpRpcStress.cs       |  2 +-
 Capnp.Net.Runtime/Rpc/TcpRpcClient.cs         | 21 +++++++---
 Capnp.Net.Runtime/Rpc/TcpRpcServer.cs         | 26 +++++++++++--
 7 files changed, 92 insertions(+), 53 deletions(-)

diff --git a/Capnp.Net.Runtime.Tests/TcpRpc.cs b/Capnp.Net.Runtime.Tests/TcpRpc.cs
index 337c1fe..a5b85b1 100644
--- a/Capnp.Net.Runtime.Tests/TcpRpc.cs
+++ b/Capnp.Net.Runtime.Tests/TcpRpc.cs
@@ -63,7 +63,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 try
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                    client.WhenConnected.Wait();
                     SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                     Assert.AreEqual(1, server.ConnectionCount);
                 }
@@ -92,7 +92,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -111,7 +111,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -129,7 +129,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -170,7 +170,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -209,7 +209,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 try
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                    client.WhenConnected.Wait();
                     SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                     Assert.AreEqual(1, server.ConnectionCount);
 
@@ -253,7 +253,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 try
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                    client.WhenConnected.Wait();
                     SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                     Assert.AreEqual(1, server.ConnectionCount);
 
@@ -312,7 +312,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -349,7 +349,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -422,7 +422,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -498,7 +498,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -615,7 +615,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
@@ -640,7 +640,17 @@ namespace Capnp.Net.Runtime.Tests
                 args2.SetStruct(1, 0);
                 args2.WriteData(0, 654321);
 
-                Assert.ThrowsException<System.ObjectDisposedException>(() => pipelined.Call(0x8765432187654321, 0x4444, args2, false));
+                try
+                {
+                    pipelined.Call(0x8765432187654321, 0x4444, args2, false);
+                    Assert.Fail("Expected an exception here");
+                }
+                catch (ObjectDisposedException)
+                {
+                }
+                catch (TaskCanceledException)
+                {
+                }
             }
         }
 
@@ -652,7 +662,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumTimeout));
+                client.WhenConnected.Wait();
                 SpinWait.SpinUntil(() => server.ConnectionCount > 0, MediumTimeout);
                 Assert.AreEqual(1, server.ConnectionCount);
 
diff --git a/Capnp.Net.Runtime.Tests/TcpRpcAdvancedStuff.cs b/Capnp.Net.Runtime.Tests/TcpRpcAdvancedStuff.cs
index 12abb9c..fee316a 100644
--- a/Capnp.Net.Runtime.Tests/TcpRpcAdvancedStuff.cs
+++ b/Capnp.Net.Runtime.Tests/TcpRpcAdvancedStuff.cs
@@ -25,7 +25,7 @@ namespace Capnp.Net.Runtime.Tests
                 {
                     using (var client = SetupClient())
                     {
-                        Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                        client.WhenConnected.Wait();
 
                         using (var main = client.GetMain<ITestInterface>())
                         {
@@ -101,7 +101,7 @@ namespace Capnp.Net.Runtime.Tests
 
                 using (var client = SetupClient())
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestInterface>())
                     {
diff --git a/Capnp.Net.Runtime.Tests/TcpRpcInterop.cs b/Capnp.Net.Runtime.Tests/TcpRpcInterop.cs
index 358bd31..949ea75 100644
--- a/Capnp.Net.Runtime.Tests/TcpRpcInterop.cs
+++ b/Capnp.Net.Runtime.Tests/TcpRpcInterop.cs
@@ -102,7 +102,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestInterface>())
                     {
@@ -152,7 +152,7 @@ namespace Capnp.Net.Runtime.Tests
 
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestPipeline>())
                     {
@@ -205,7 +205,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
@@ -271,7 +271,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
@@ -361,7 +361,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestTailCaller>())
                     {
@@ -417,7 +417,7 @@ namespace Capnp.Net.Runtime.Tests
 
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
@@ -462,7 +462,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
@@ -528,7 +528,7 @@ namespace Capnp.Net.Runtime.Tests
 
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
@@ -621,7 +621,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     var destructionPromise = new TaskCompletionSource<int>();
                     var destructionTask = destructionPromise.Task;
@@ -679,7 +679,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
@@ -836,7 +836,7 @@ namespace Capnp.Net.Runtime.Tests
             using (var client = new TcpRpcClient("localhost", TcpPort))
             {
 
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 using (var main = client.GetMain<ITestMoreStuff>())
                 {
@@ -929,7 +929,7 @@ namespace Capnp.Net.Runtime.Tests
                 label:
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
@@ -1002,7 +1002,7 @@ namespace Capnp.Net.Runtime.Tests
             {
                 using (var client = new TcpRpcClient("localhost", TcpPort))
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     using (var main = client.GetMain<ITestMoreStuff>())
                     {
diff --git a/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs b/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs
index bd1f269..01658a8 100644
--- a/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs
+++ b/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs
@@ -22,7 +22,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 server.Main = new TestInterfaceImpl(counters);
@@ -52,7 +52,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 server.Main = new TestPipelineImpl(counters);
@@ -87,7 +87,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 server.Main = new TestMoreStuffImpl(counters);
@@ -120,7 +120,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 server.Main = new TestMoreStuffImpl(counters);
@@ -159,7 +159,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 server.Main = new TestTailCallerImpl(counters);
@@ -196,7 +196,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 server.Main = new TestMoreStuffImpl(counters);
@@ -226,7 +226,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 var impl = new TestMoreStuffImpl(counters);
@@ -268,7 +268,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var destructionPromise = new TaskCompletionSource<int>();
                 var destructionTask = destructionPromise.Task;
@@ -345,7 +345,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var destructionPromise = new TaskCompletionSource<int>();
                 var destructionTask = destructionPromise.Task;
@@ -388,7 +388,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var destructionPromise = new TaskCompletionSource<int>();
                 var destructionTask = destructionPromise.Task;
@@ -435,7 +435,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 var impl = new TestMoreStuffImpl(counters);
@@ -496,7 +496,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 var impl = new TestMoreStuffImpl(counters);
@@ -553,7 +553,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 var impl = new TestMoreStuffImpl(counters);
@@ -590,7 +590,7 @@ namespace Capnp.Net.Runtime.Tests
             using (server)
             using (client)
             {
-                Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                client.WhenConnected.Wait();
 
                 var counters = new Counters();
                 var impl = new TestMoreStuffImpl(counters);
diff --git a/Capnp.Net.Runtime.Tests/TcpRpcStress.cs b/Capnp.Net.Runtime.Tests/TcpRpcStress.cs
index d8ae9cc..442df94 100644
--- a/Capnp.Net.Runtime.Tests/TcpRpcStress.cs
+++ b/Capnp.Net.Runtime.Tests/TcpRpcStress.cs
@@ -43,7 +43,7 @@ namespace Capnp.Net.Runtime.Tests
                 using (server)
                 using (client)
                 {
-                    Assert.IsTrue(client.WhenConnected.Wait(MediumNonDbgTimeout));
+                    client.WhenConnected.Wait();
 
                     var counters = new Counters();
                     var impl = new TestMoreStuffImpl(counters);
diff --git a/Capnp.Net.Runtime/Rpc/TcpRpcClient.cs b/Capnp.Net.Runtime/Rpc/TcpRpcClient.cs
index 6e3bfa4..cf9c0ab 100644
--- a/Capnp.Net.Runtime/Rpc/TcpRpcClient.cs
+++ b/Capnp.Net.Runtime/Rpc/TcpRpcClient.cs
@@ -55,13 +55,22 @@ namespace Capnp.Rpc
 
         async Task ConnectAsync(string host, int port)
         {
-            try
+            for (int retry = 0; ; retry++)
             {
-                await _client.ConnectAsync(host, port);
-            }
-            catch (SocketException exception)
-            {
-                throw new RpcException("TcpRpcClient is unable to connect", exception);
+                try
+                {
+                    await _client.ConnectAsync(host, port);
+
+                    return;
+                }
+                catch (SocketException exception) when (retry < 240 && exception.SocketErrorCode == SocketError.AddressAlreadyInUse)
+                {
+                    await Task.Delay(1000);
+                }
+                catch (SocketException exception)
+                {
+                    throw new RpcException("TcpRpcClient is unable to connect", exception);
+                }
             }
         }
 
diff --git a/Capnp.Net.Runtime/Rpc/TcpRpcServer.cs b/Capnp.Net.Runtime/Rpc/TcpRpcServer.cs
index e1f6586..0eee043 100644
--- a/Capnp.Net.Runtime/Rpc/TcpRpcServer.cs
+++ b/Capnp.Net.Runtime/Rpc/TcpRpcServer.cs
@@ -169,9 +169,16 @@ namespace Capnp.Rpc
                 }
             }
 
-            if (!_acceptorThread.Join(500))
+            try
             {
-                Logger.LogError("Unable to join TCP acceptor thread within timeout");
+                if (!_acceptorThread.Join(500))
+                {
+                    Logger.LogError("Unable to join TCP acceptor thread within timeout");
+                }
+            }
+            catch (ThreadStateException)
+            {
+                // If acceptor thread was not yet started this is not a problem. Ignore.
             }
 
             GC.SuppressFinalize(this);
@@ -189,7 +196,20 @@ namespace Capnp.Rpc
             _rpcEngine = new RpcEngine();
             _listener = new TcpListener(localAddr, port);
             _listener.ExclusiveAddressUse = false;
-            _listener.Start();
+
+            for (int retry = 0; retry < 5; retry++)
+            {
+                try
+                {
+                    _listener.Start();
+                    break;
+                }
+                catch (SocketException socketException)
+                {
+                    Logger.LogWarning($"Failed to listen on port {port}, attempt {retry}: {socketException}");
+                    Thread.Sleep(10);
+                }
+            }
 
             _acceptorThread = new Thread(AcceptClients);