aboutsummaryrefslogtreecommitdiffstats
path: root/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
diff options
context:
space:
mode:
authorjakob <jakob@eggerapps.at>2010-04-16 17:18:54 +0000
committerjakob <jakob@eggerapps.at>2010-04-16 17:18:54 +0000
commitf5f15a660b5663865347e2b2b162fba7ad86566f (patch)
tree5c8f1093381e02563f4b816c96a6848bf9efac3d /Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
parent4c7dbcd724e9ac6fbd5943f147b15472833dceeb (diff)
downloadsequelpro-f5f15a660b5663865347e2b2b162fba7ad86566f.tar.gz
sequelpro-f5f15a660b5663865347e2b2b162fba7ad86566f.tar.bz2
sequelpro-f5f15a660b5663865347e2b2b162fba7ad86566f.zip
- changed the query locking mechanism for MCPConnection to be more thread safe. From now on, always use [self lockConnection] rather than [queryLock lock], independent of what thread you are running on
- A warning is written to the console when the connection is unlocked multiple times (to identify potential race conditions) - modified MCPStreamingResult to ensure it only closes the connection once - added a check to prevent arrow key navigation past the last row
Diffstat (limited to 'Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m')
-rw-r--r--Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m138
1 files changed, 83 insertions, 55 deletions
diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
index ad2f72e4..a94584ee 100644
--- a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
+++ b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
@@ -90,9 +90,12 @@ static BOOL sTruncateLongFieldInLogs = YES;
mEncoding = NSISOLatin1StringEncoding;
mConnectionFlags = kMCPConnectionDefaultOption;
-
- queryLock = [[NSLock alloc] init];
-
+
+ // Anything that performs a mysql_net_read is not thread-safe: mysql queries, pings
+ // Always lock the connection first. Don't use this lock directly, use the lockConnection method!
+ connectionLock = [[NSConditionLock alloc] initWithCondition:MCPConnectionIdle];
+ [connectionLock setName:@"MCPConnection connectionLock"];
+
connectionHost = nil;
connectionLogin = nil;
connectionSocket = nil;
@@ -642,7 +645,7 @@ static BOOL sTruncateLongFieldInLogs = YES;
- (BOOL)pingConnection
{
// Set up a query lock
- [queryLock lock];
+ [self lockConnection];
uint64_t currentTime_t;
Nanoseconds elapsedTime;
@@ -671,7 +674,7 @@ static BOOL sTruncateLongFieldInLogs = YES;
}
pthread_attr_destroy(&attr);
- [queryLock unlock];
+ [self unlockConnection];
return lastPingSuccess;
}
@@ -708,10 +711,11 @@ void pingConnectionTask(void *ptr)
return;
}
- // Attempt to get a query lock, but release it to ensure the connection isn't locked
- // by a background ping.
- if (![queryLock tryLock]) return;
- [queryLock unlock];
+ // Attempt to lock the connection. If the connection currently is busy,
+ // we don't need a ping. The connection is unlocked in threadedKeepAlive
+ // before the ping actually returns, to prevent the ping from delaying
+ // other queries
+ if (![self tryLockConnection]) return;
// Store the ping time
lastKeepAliveTime = timeConnected;
@@ -726,7 +730,11 @@ void pingConnectionTask(void *ptr)
*/
- (void)threadedKeepAlive
{
- if (!mConnected || keepAliveThread != NULL) return;
+ if (!mConnected || keepAliveThread != NULL) {
+ // unlock the connection now. it has been locked in keepAlive:
+ [self unlockConnection];
+ return;
+ }
// Use a ping timeout between zero and thirty seconds
NSInteger pingTimeout = 30;
@@ -738,6 +746,9 @@ void pingConnectionTask(void *ptr)
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
pthread_create(&keepAliveThread, &attr, (void *)&performThreadedKeepAlive, (void *)mConnection);
+ // unlock the connection now. it has been locked in keepAlive:
+ [self unlockConnection];
+
// Give the connection time to respond, but force a timeout after the ping timeout
// if the thread hasn't already closed itself.
sleep(pingTimeout);
@@ -1129,13 +1140,12 @@ void performThreadedKeepAlive(void *ptr)
if (mConnected) {
const char *theDBName = [self cStringFromString:dbName];
- [queryLock lock];
+ [self lockConnection];
if (0 == mysql_select_db(mConnection, theDBName)) {
- [queryLock unlock];
-
+ [self unlockConnection];
return YES;
}
- [queryLock unlock];
+ [self unlockConnection];
}
[self setLastErrorMessage:nil];
@@ -1444,10 +1454,7 @@ void performThreadedKeepAlive(void *ptr)
}
}
- // Lock the connection - on this thread for normal result sets (avoiding blocking issues
- // when the app is in modal mode), or ensuring a lock on the main thread for streaming queries.
- if (streamResultType == MCPStreamingNone) [queryLock lock];
- else [self lockConnection];
+ [self lockConnection];
// Run (or re-run) the query, timing the execution time of the query - note
// that this time will include network lag.
@@ -1466,7 +1473,7 @@ void performThreadedKeepAlive(void *ptr)
// For normal result sets, fetch the results and unlock the connection
if (streamResultType == MCPStreamingNone) {
theResult = [[MCPResult alloc] initWithMySQLPtr:mConnection encoding:mEncoding timeZone:mTimeZone];
- if (!queryCancelled || !queryCancelUsedReconnect) [queryLock unlock];
+ if (!queryCancelled || !queryCancelUsedReconnect) [self unlockConnection];
// For streaming result sets, fetch the result pointer and leave the connection locked
} else if (streamResultType == MCPStreamingFast) {
@@ -1482,8 +1489,7 @@ void performThreadedKeepAlive(void *ptr)
break;
}
} else {
- if (streamResultType == MCPStreamingNone) [queryLock unlock];
- else [self unlockConnection];
+ [self unlockConnection];
}
queryErrorMessage = [[NSString alloc] initWithString:@""];
@@ -1495,8 +1501,7 @@ void performThreadedKeepAlive(void *ptr)
// On failure, set the error messages and IDs
} else {
if (!queryCancelled || !queryCancelUsedReconnect) {
- if (streamResultType == MCPStreamingNone) [queryLock unlock];
- else [self unlockConnection];
+ [self unlockConnection];
}
if (queryCancelled) {
@@ -1586,8 +1591,8 @@ void performThreadedKeepAlive(void *ptr)
if (![self isConnected]) return;
// Check whether a query is actually being performed - if not, also return.
- if ([queryLock tryLock]) {
- [queryLock unlock];
+ if ([self tryLockConnection]) {
+ [self unlockConnection];
return;
}
@@ -1677,32 +1682,55 @@ void performThreadedKeepAlive(void *ptr)
#pragma mark Connection locking
/**
- * Lock the connection from any thread; ensure the the connection is locked on
- * the main thread, but as fast as possible.
+ * Lock the connection. This must be done before performing any operation
+ * that is not thread safe, eg. performing queries or pinging.
*/
- (void)lockConnection
{
- if ([NSThread isMainThread]) [queryLock lock];
- else [queryLock performSelectorOnMainThread:@selector(lock) withObject:nil waitUntilDone:YES];
+ // We can only start a query as soon as the condition is MCPConnectionIdle
+ [connectionLock lockWhenCondition:MCPConnectionIdle];
+
+ // We now set the condition to MCPConnectionBusy
+ [connectionLock unlockWithCondition:MCPConnectionBusy];
}
/**
- * Unlock the connection from any thread; ensure the connection is unlocked on
- * the main thread, but as fast as possible.
+ * Try locking the connection. If the connection is idle (unlocked), this method
+ * locks the connection and returns YES. The connection must afterwards be unlocked
+ * using unlockConnection. If the connection is currently busy (locked), this
+ * method immediately returns NO and doesn't lock the connection.
*/
-- (void)unlockConnection
+- (BOOL)tryLockConnection
{
+ // check if the condition is MCPConnectionIdle
+ if ([connectionLock tryLockWhenCondition:MCPConnectionIdle]) {
+ // We're allowed to use the connection!
+ [connectionLock unlockWithCondition:MCPConnectionBusy];
+ return YES;
+ } else {
+ // Someone else is using the connection right now
+ return NO;
+ }
+}
- // Ensure the unlock occurs on the main thread
- if (![NSThread isMainThread]) {
- [self performSelectorOnMainThread:@selector(unlockConnection) withObject:nil waitUntilDone:NO];
- return;
- }
- // Unlock the connection, first ensuring it is locked to avoid
- // multiple unlock call issues (eg reconnected queries, threading)
- [queryLock tryLock];
- [queryLock unlock];
+/**
+ * Unlock the connection.
+ */
+- (void)unlockConnection
+{
+ // We don't care if the connection is busy or not
+ [connectionLock lock];
+
+ // We check if the connection actually was busy. If it wasn't busy,
+ // it means we probably tried to unlock the connection twice. This is
+ // potentially dangerous, therefore we log this to the console
+ if ([connectionLock condition]!=MCPConnectionBusy) {
+ NSLog(@"Tried to unlock the connection, but it wasn't locked.");
+ }
+
+ // We tell everyone that the connection is available again!
+ [connectionLock unlockWithCondition:MCPConnectionIdle];
}
#pragma mark -
@@ -1731,7 +1759,7 @@ void performThreadedKeepAlive(void *ptr)
if (![self checkConnection]) return [[[MCPResult alloc] init] autorelease];
- [queryLock lock];
+ [self lockConnection];
if ((dbsName == nil) || ([dbsName isEqualToString:@""])) {
if (theResPtr = mysql_list_dbs(mConnection, NULL)) {
theResult = [[MCPResult alloc] initWithResPtr: theResPtr encoding: mEncoding timeZone:mTimeZone];
@@ -1750,7 +1778,7 @@ void performThreadedKeepAlive(void *ptr)
theResult = [[MCPResult alloc] init];
}
}
- [queryLock unlock];
+ [self unlockConnection];
if (theResult) {
[theResult autorelease];
@@ -1784,7 +1812,7 @@ void performThreadedKeepAlive(void *ptr)
if (![self checkConnection]) return [[[MCPResult alloc] init] autorelease];
- [queryLock lock];
+ [self lockConnection];
if ((tablesName == nil) || ([tablesName isEqualToString:@""])) {
if (theResPtr = mysql_list_tables(mConnection, NULL)) {
theResult = [[MCPResult alloc] initWithResPtr: theResPtr encoding: mEncoding timeZone:mTimeZone];
@@ -1803,7 +1831,7 @@ void performThreadedKeepAlive(void *ptr)
}
}
- [queryLock unlock];
+ [self unlockConnection];
if (theResult) {
[theResult autorelease];
@@ -2375,14 +2403,14 @@ void performThreadedKeepAlive(void *ptr)
MCPResult *theResult = nil;
MYSQL_RES *theResPtr;
- [queryLock lock];
+ [self lockConnection];
if (theResPtr = mysql_list_processes(mConnection)) {
theResult = [[MCPResult alloc] initWithResPtr:theResPtr encoding:mEncoding timeZone:mTimeZone];
}
else {
theResult = [[MCPResult alloc] init];
}
- [queryLock unlock];
+ [self unlockConnection];
if (theResult) {
[theResult autorelease];
@@ -2547,7 +2575,7 @@ void performThreadedKeepAlive(void *ptr)
if ([self serverMajorVersion] == 3) queryString = "SHOW VARIABLES LIKE 'max_allowed_packet'";
else queryString = "SELECT @@global.max_allowed_packet";
- [queryLock lock];
+ [self lockConnection];
if (0 == mysql_query(mConnection, queryString)) {
if (mysql_field_count(mConnection) != 0) {
MCPResult *r = [[MCPResult alloc] initWithMySQLPtr:mConnection encoding:mEncoding timeZone:mTimeZone];
@@ -2555,13 +2583,13 @@ void performThreadedKeepAlive(void *ptr)
NSArray *a = [r fetchRowAsArray];
[r autorelease];
if([a count]) {
- [queryLock unlock];
+ [self unlockConnection];
maxAllowedPacketSize = [[a objectAtIndex:([self serverMajorVersion] == 3)?1:0] integerValue];
return true;
}
}
}
- [queryLock unlock];
+ [self unlockConnection];
return false;
}
@@ -2596,9 +2624,9 @@ void performThreadedKeepAlive(void *ptr)
{
if(![self isMaxAllowedPacketEditable] || newSize < 1024) return maxAllowedPacketSize;
- [queryLock lock];
+ [self lockConnection];
mysql_query(mConnection, [[NSString stringWithFormat:@"SET GLOBAL max_allowed_packet = %ld", newSize] UTF8String]);
- [queryLock unlock];
+ [self unlockConnection];
// Inform the user via a log entry about that change according to reset value
if(delegate && [delegate respondsToSelector:@selector(queryGaveError:connection:)])
@@ -2617,9 +2645,9 @@ void performThreadedKeepAlive(void *ptr)
{
BOOL isEditable;
- [queryLock lock];
+ [self lockConnection];
isEditable = !mysql_query(mConnection, "SET GLOBAL max_allowed_packet = @@global.max_allowed_packet");
- [queryLock unlock];
+ [self unlockConnection];
return isEditable;
}
@@ -2753,7 +2781,7 @@ void performThreadedKeepAlive(void *ptr)
// Ensure the query lock is unlocked, thereafter setting to nil in case of pending calls
[self unlockConnection];
- [queryLock release], queryLock = nil;
+ [connectionLock release], connectionLock = nil;
// Clean up connections if necessary
if (mConnected) [self disconnect];