Merge pull request #129 from PashaPash/connect-timeout-fix

Fixed connect timeout handling in ConnectionMultiplexer.Connect

StackExchange.Redis.Tests/ConnectToUnexistingHost.cs

+using NUnit.Framework;
+using System;
+using System.Diagnostics;
+using System.Threading;
+
+namespace StackExchange.Redis.Tests
+{
+    [TestFixture]
+    public class ConnectToUnexistingHost : TestBase
+    {
+#if DEBUG
+        [Test]
+        [TestCase(CompletionType.Any)]
+        [TestCase(CompletionType.Sync)]
+        [TestCase(CompletionType.Async)]
+        public void ConnectToUnexistingHostFailsWithinTimeout(CompletionType completionType)
+        {
+            var sw = Stopwatch.StartNew();
+
+            try
+            {
+                var config = new ConfigurationOptions
+                {
+                    EndPoints = { { "invalid", 1234 } },
+                    ConnectTimeout = 1000
+                };
+
+                SocketManager.ConnectCompletionType = completionType;
+
+                using (var muxer = ConnectionMultiplexer.Connect(config))
+                {
+                    Thread.Sleep(10000);
+                }
+
+                Assert.Fail("Connect should fail with RedisConnectionException exception");
+            }
+            catch (RedisConnectionException)
+            {
+                var elapsed = sw.ElapsedMilliseconds;
+                if (elapsed > 9000) 
+                {
+                    Assert.Fail("Connect should fail within ConnectTimeout");
+                }
+            }
+            finally
+            {
+                SocketManager.ConnectCompletionType = CompletionType.Any;
+            }
+        }
+#endif
+    }
+}
\ No newline at end of file

StackExchange.Redis.Tests/StackExchange.Redis.Tests.csproj

@@ -67,6 +67,7 @@
     <Compile Include="AsyncTests.cs" />
     <Compile Include="BasicOps.cs" />
     <Compile Include="ConnectingFailDetection.cs" />
+    <Compile Include="ConnectToUnexistingHost.cs" />
     <Compile Include="HyperLogLog.cs" />
     <Compile Include="WrapperBaseTests.cs" />
     <Compile Include="TransactionWrapperTests.cs" />

StackExchange.Redis/StackExchange/Redis/ConnectionMultiplexer.cs

@@ -718,48 +718,34 @@ static ConnectionMultiplexer CreateMultiplexer(object configuration)
         /// </summary>
         public static ConnectionMultiplexer Connect(string configuration, TextWriter log = null)
         {
-            IDisposable killMe = null;
-            try
-            {
-                var muxer = CreateMultiplexer(configuration);
-                killMe = muxer;
-                // note that task has timeouts internally, so it might take *just over* the reegular timeout
-                var task = muxer.ReconfigureAsync(true, false, log, null, "connect");
-                if (!task.Wait(muxer.SyncConnectTimeout(true)))
-                {
-                    task.ObserveErrors();
-                    if (muxer.RawConfig.AbortOnConnectFail)
-                    {
-                        throw new TimeoutException();
-                    }
-                }
-                if(!task.Result) throw ExceptionFactory.UnableToConnect(muxer.failureMessage);
-                killMe = null;
-                return muxer;
-            }
-            finally
-            {
-                if (killMe != null) try { killMe.Dispose(); } catch { }
-            }            
+            return ConnectImpl(() => CreateMultiplexer(configuration), log);
         }
+
         /// <summary>
         /// Create a new ConnectionMultiplexer instance
         /// </summary>
         public static ConnectionMultiplexer Connect(ConfigurationOptions configuration, TextWriter log = null)
+        {
+            return ConnectImpl(() => CreateMultiplexer(configuration), log);
+        }
+
+        private static ConnectionMultiplexer ConnectImpl(Func<ConnectionMultiplexer> multiplexerFactory, TextWriter log)
         {
             IDisposable killMe = null;
             try
             {
-                var muxer = CreateMultiplexer(configuration);
+                var muxer = multiplexerFactory();
                 killMe = muxer;
-                // note that task has timeouts internally, so it might take *just over* the reegular timeout
-                var task = muxer.ReconfigureAsync(true, false, log, null, "connect");
+                // note that task has timeouts internally, so it might take *just over* the regular timeout
+                // wrap into task to force async execution
+                var task = Task.Factory.StartNew(() => { return muxer.ReconfigureAsync(true, false, log, null, "connect").Result; });
+
                 if (!task.Wait(muxer.SyncConnectTimeout(true)))
                 {
                     task.ObserveErrors();
                     if (muxer.RawConfig.AbortOnConnectFail)
                     {
-                        throw new TimeoutException();
+                        throw ExceptionFactory.UnableToConnect("Timeout");
                     }
                 }
                 if (!task.Result) throw ExceptionFactory.UnableToConnect(muxer.failureMessage);

StackExchange.Redis/StackExchange/Redis/SocketManager.cs

@@ -130,7 +130,7 @@ internal SocketToken BeginConnect(EndPoint endpoint, ISocketCallback callback)
                 CompletionTypeHelper.RunWithCompletionType(
                     (cb) => socket.BeginConnect(endpoint, cb, Tuple.Create(socket, callback)),
                     (ar) => EndConnectImpl(ar),
-                    CompletionType.Sync);
+                    connectCompletionType);
             } 
             catch (NotImplementedException ex)
             {