Significantly reduce risk of a race condition in a certain setup (see #2107)

* Replaced a run loop sleep with a thread sleep as it was on a background thread where a) a regular sleep is fine, b) the run loop sleep did not seem to have any effect
* While we are at it, cut done some duplicate code
* Removed a redundant if()

2 files changed, 34 insertions, 32 deletions

diff --git a/Source/SPConnectionHandler.m b/Source/SPConnectionHandler.m
index 80e9c3f5..c216142f 100644
--- a/Source/SPConnectionHandler.m
+++ b/Source/SPConnectionHandler.m
@@ -174,8 +174,12 @@ static NSString *SPLocalhostAddress = @"127.0.0.1";
 	if (![mySQLConnection isConnected]) {
 		if (sshTunnel && !cancellingConnection) {
 			
-			// If an SSH tunnel is running, temporarily block to allow the tunnel to register changes in state
-			[[NSRunLoop currentRunLoop] runMode:NSModalPanelRunLoopMode beforeDate:[NSDate dateWithTimeIntervalSinceNow:0.2]];
+			// This is a race condition we cannot fix "properly":
+			// For meaningful error handling we need to also consider the debug output from the SSH connection.
+			// The SSH debug output might be sligthly delayed though (flush, delegates, ...) or
+			// there might not even by any output at all (when it is purely a libmysql issue).
+			// TL;DR: No guaranteed events we could wait for, just trying our luck.
+			[NSThread sleepForTimeInterval:0.1]; // 100ms
 			
 			// If the state is connection refused, attempt the MySQL connection again with the host using the hostfield value.
 			if ([sshTunnel state] == SPMySQLProxyForwardingFailed) {
@@ -184,7 +188,7 @@ static NSString *SPLocalhostAddress = @"127.0.0.1";
 					[mySQLConnection connect];
 					
 					if (![mySQLConnection isConnected]) {
-						[[NSRunLoop currentRunLoop] runMode:NSModalPanelRunLoopMode beforeDate:[NSDate dateWithTimeIntervalSinceNow:0.2]];
+						[NSThread sleepForTimeInterval:0.1]; //100ms
 					}
 				}
 			}
diff --git a/Source/SPSSHTunnel.m b/Source/SPSSHTunnel.m
index 8e9ffb13..d2c999c4 100644
--- a/Source/SPSSHTunnel.m
+++ b/Source/SPSSHTunnel.m
@@ -40,6 +40,8 @@
 #import <netinet/in.h>
 #import <CommonCrypto/CommonDigest.h>
 
+static unsigned short getRandomPort();
+
 @implementation SPSSHTunnel
 
 @synthesize passwordPromptCancelled;
@@ -264,36 +266,10 @@
 
 	// If no local port has yet been chosen, choose one
 	if (!localPort) {
-		int tempSocket;
-		struct sockaddr_in tempSocketAddress;
-		size_t addressLength = sizeof(tempSocketAddress);
-		if((tempSocket = socket(AF_INET, SOCK_STREAM, 0)) > 0) {
-			memset(&tempSocketAddress, 0, sizeof(tempSocketAddress));
-			tempSocketAddress.sin_family = AF_INET;
-			tempSocketAddress.sin_addr.s_addr = htonl(INADDR_ANY);
-			tempSocketAddress.sin_port = 0;
-			if (bind(tempSocket, (struct sockaddr *)&tempSocketAddress, (socklen_t)addressLength) >= 0) {
-				if (getsockname(tempSocket, (struct sockaddr *)&tempSocketAddress, (uint32_t *)&addressLength) >= 0) {
-					localPort = ntohs(tempSocketAddress.sin_port);
-				}
-			}
-			close(tempSocket);
-		}
+		localPort = getRandomPort();
 		
 		if (useHostFallback) {
-			if((tempSocket = socket(AF_INET, SOCK_STREAM, 0)) > 0) {
-				memset(&tempSocketAddress, 0, sizeof(tempSocketAddress));
-				tempSocketAddress.sin_family = AF_INET;
-				tempSocketAddress.sin_addr.s_addr = htonl(INADDR_ANY);
-				tempSocketAddress.sin_port = 0;
-				if (bind(tempSocket, (struct sockaddr *)&tempSocketAddress, (socklen_t)addressLength) >= 0) {
-					if (getsockname(tempSocket, (struct sockaddr *)&tempSocketAddress, (uint32_t *)&addressLength) >= 0) {
-						localPortFallback = ntohs(tempSocketAddress.sin_port);
-					}
-				}
-				close(tempSocket);
-			}
-		
+			localPortFallback = getRandomPort();
 		}
 		
 		// Abort if no local free port could be allocated
@@ -748,7 +724,7 @@
 {
 	delegate = nil;
 	[[NSNotificationCenter defaultCenter] removeObserver:self];
-	if (connectionState != SPMySQLProxyIdle) [self disconnect];
+	[self disconnect];
 	[NSObject cancelPreviousPerformRequestsWithTarget:self];
 	SPClear(sshHost);
 	SPClear(sshLogin);
@@ -775,3 +751,25 @@
 }
 
 @end
+
+#pragma mark -
+
+unsigned short getRandomPort() {
+	int port = 0;
+	int tempSocket;
+	struct sockaddr_in tempSocketAddress;
+	size_t addressLength = sizeof(tempSocketAddress);
+	if((tempSocket = socket(AF_INET, SOCK_STREAM, 0)) > 0) {
+		memset(&tempSocketAddress, 0, sizeof(tempSocketAddress));
+		tempSocketAddress.sin_family = AF_INET;
+		tempSocketAddress.sin_addr.s_addr = htonl(INADDR_ANY);
+		tempSocketAddress.sin_port = 0;
+		if (bind(tempSocket, (struct sockaddr *)&tempSocketAddress, (socklen_t)addressLength) >= 0) {
+			if (getsockname(tempSocket, (struct sockaddr *)&tempSocketAddress, (uint32_t *)&addressLength) >= 0) {
+				port = ntohs(tempSocketAddress.sin_port);
+			}
+		}
+		close(tempSocket);
+	}
+	return port;
+}