Commits

Tuukka Norri committed f2923ef

Partial thread safety in PostgreSQL connector
- Added thread checks to PGTSAsynchronousConnector.
- Some of its methods are now thread safe but making the connection isn't.
- Changed a method name to be grammatically correct and easier to search.

  • Participants
  • Parent commits 29dfefa

Comments (0)

Files changed (5)

Sources/PGTSAsynchronousConnector.m

 #import <arpa/inet.h>
 
 
-@implementation PGTSAsynchronousConnector
 static void 
 SocketReady (CFSocketRef s, CFSocketCallBackType callBackType, CFDataRef address, const void* data, void* self)
 {
 }
 
 
+
+@implementation PGTSAsynchronousConnector
 - (id) init
 {
 	if ((self = [super init]))
 
 - (void) setCFRunLoop: (CFRunLoopRef) aRef
 {
-	if (mRunLoop != aRef)
+	@synchronized (self)
 	{
-		if (mRunLoop) CFRelease (mRunLoop);
-		if (aRef)
+		if (mRunLoop != aRef)
 		{
+			if (mRunLoop)
+				CFRelease (mRunLoop);
+			
 			mRunLoop = aRef;
-			CFRetain (mRunLoop);
+			
+			if (mRunLoop)
+				CFRetain (mRunLoop);
 		}
 	}
 }
 
 - (void) setConnectionDictionary: (NSDictionary *) aDict
 {
-	if (mConnectionDictionary != aDict)
+	@synchronized (self)
 	{
-		[mConnectionDictionary release];
-		mConnectionDictionary = [aDict retain];
+		if (mConnectionDictionary != aDict)
+		{
+			[mConnectionDictionary release];
+			mConnectionDictionary = [aDict retain];
+		}
 	}
 }
 
 	[self freeCFTypes];
 	[self cancel];
 	[mConnectionError release];
+	[mConnectionDictionary release];
+	[mHostResolver release];
 	[super dealloc];
 }
 
 }
 
 
-- (void) prepareForConnect
+- (void) prepareToConnect
 {
-	[super prepareForConnect];
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
+	[super prepareToConnect];
 	bzero (&mHostError, sizeof (mHostError));
 }
 
 
 - (void) hostResolverDidSucceed: (BXHostResolver *) resolver addresses: (NSArray *) addresses
 {
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
 	BOOL reachedServer = NO;
 	if (addresses)
 	{
 
 - (void) hostResolverDidFail: (BXHostResolver *) resolver error: (NSError *) error
 {
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
 	[self setConnectionError: error];		
 	[self finishedConnecting: NO];
 }
 - (void) socketReady: (CFSocketCallBackType) callBackType
 {
 	BXLogDebug (@"Socket got ready.");
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
 	
 	//Sometimes the wrong callback type gets called. We cope with this
 	//by checking against an expected type and re-enabling it if needed.
 
 - (void) finishedConnecting: (BOOL) succeeded
 {
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
 	[self freeCFTypes];
 	[super finishedConnecting: succeeded];
 }
 - (BOOL) connect: (NSDictionary *) connectionDictionary
 {
 	BXLogDebug (@"Beginning connecting.");
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
 	
 	BOOL retval = NO;
 	mExpectedCallBack = 0;
-	[self prepareForConnect];
+	[self prepareToConnect];
 	[self setConnectionDictionary: connectionDictionary];
 	
 	//CFSocket etc. do some nice things for us that prevent libpq from noticing
 - (BOOL) startNegotiation: (const char *) conninfo
 {
 	BXLogDebug (@"Beginning negotiation.");
-	
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
+
 	mNegotiationStarted = NO;
 	BOOL retval = NO;
 	if ([self start: conninfo])
 - (void) negotiateConnection
 {
 	BXLogDebug (@"Negotiating.");
-	
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
+
 	if (mTraceFile)
 		PQtrace (mConnection, mTraceFile);
 	
 
 - (BOOL) start: (const char *) connectionString
 {
+	ExpectL (CFRunLoopGetCurrent () == mRunLoop);
 	return (BOOL) PQresetStart (mConnection);
 }
 @end

Sources/PGTSConnection.mm

 
 #import "NSString+PGTSAdditions.h"
 
-// FIXME: change connector-related methods so that they Expect (mConnector) and synchronize on the connector, or make the connector thread-safe.
-// FIXME: See that methods don't pass self while synchronized to anything that calls -executeQuery:….
 
 
 @interface PGTSConnection (PGTSConnectorDelegate) <PGTSConnectorDelegate>
 
 
 
-@implementation PGTSConnection
 static void
 NoticeReceiver (void *connectionPtr, PGresult const *notice)
 {
 	PGTSConnection *connection = (PGTSConnection *) connectionPtr;
 	NSError *error = [PGTSResultSet errorForPGresult: notice];
-	[connection->mDelegate PGTSConnection: connection receivedNotice: error];
+	[[connection delegate] PGTSConnection: connection receivedNotice: error];
 }
 
 
+
+/**
+ * \internal
+ * \brief A connection class for Postgresql.
+ * \note Instances of this class may be used from multiple threads after a connection has been made.
+ */
+@implementation PGTSConnection
 + (void) initialize
 {
 	static BOOL tooLate = NO;
 		}
 		
 		// Send the actual query.
-		// FIXME: move outside @synchronized
 		PGTSQueryDescription* desc = [PGTSQueryDescription queryDescriptionFor: queryString 
 																	  delegate: nil
 																	  callback: NULL 
 	PQsetClientEncoding (connection, "UNICODE"); 
 	
 	BOOL shouldContinue = YES;
-	const char* queries [] = {
+	char const * const queries [] = {
 		"SET standard_conforming_strings TO true",
 		"SET datestyle TO 'ISO, YMD'",
 		"SET timezone TO 'UTC'",
 	else
 	{
 		[[BXConnectionMonitor sharedInstance] clientDidFailConnectionAttempt: self];
+        [mDelegate PGTSConnectionFailed: self];
 	}
 }
 

Sources/PGTSConnector.h

 
 - (void) finishedConnecting: (BOOL) status;
 - (void) setUpSSL;
-- (void) prepareForConnect;
+- (void) prepareToConnect;
 @end

Sources/PGTSConnector.m

 static int
 SSLConnectionExIndex ()
 {
-	static int sslConnectionExIndex = -1;
+	static volatile int sslConnectionExIndex = -1;
 	if (-1 == sslConnectionExIndex)
 		sslConnectionExIndex = SSL_get_ex_new_index (0, NULL, NULL, NULL, NULL);
 	return sslConnectionExIndex;
 }
 
 
-- (void) prepareForConnect
+- (void) prepareToConnect
 {
 	mSSLSetUp = NO;
 	mNegotiationStarted = NO;

Sources/PGTSSynchronousConnector.m

 	//Here libpq can resolve the name for us, because we don't use CFRunLoop and CFSocket.
 
     BOOL retval = NO;
-	[self prepareForConnect];
+	[self prepareToConnect];
 	char* conninfo = PGTSCopyConnectionString (connectionDictionary);
 	if ([self start: conninfo] && CONNECTION_BAD != PQstatus (mConnection))
 	{