Fixing potential deadlock situation involving modules and autoDelegates.

Author: robbiehanson

Core/XMPPInternal.h

@@ -2,7 +2,8 @@
 //  This file is for XMPPStream and various internal components.
 //
 
-#import "XMPPSASLAuthentication.h"
+#import "XMPPStream.h"
+#import "XMPPModule.h"
 
 // Define the various states we'll use to track our progress
 enum XMPPStreamState
@@ -60,3 +61,14 @@ extern NSString *const XMPPStreamDidChangeMyJIDNotification;
 - (void)injectElement:(NSXMLElement *)element;
 
 @end
+
+@interface XMPPModule (/* Internal */)
+
+/**
+ * Used internally by methods like XMPPStream's unregisterModule:.
+ * Normally removing a delegate is a synchronous operation, but due to multiple dispatch_sync operations,
+ * it must occasionally be done asynchronously to avoid deadlock.
+**/
+- (void)removeDelegate:(id)delegate delegateQueue:(dispatch_queue_t)delegateQueue synchronously:(BOOL)synchronously;
+
+@end

Core/XMPPModule.m

@@ -178,36 +178,32 @@ - (void)addDelegate:(id)delegate delegateQueue:(dispatch_queue_t)delegateQueue
 		dispatch_async(moduleQueue, block);
 }
 
-- (void)removeDelegate:(id)delegate delegateQueue:(dispatch_queue_t)delegateQueue
+- (void)removeDelegate:(id)delegate delegateQueue:(dispatch_queue_t)delegateQueue synchronously:(BOOL)synchronously
 {
-	// Synchronous operation
-	// 
-	// Delegate removal MUST always be synchronous.
-	
 	dispatch_block_t block = ^{
 		[multicastDelegate removeDelegate:delegate delegateQueue:delegateQueue];
 	};
 
 	if (dispatch_get_current_queue() == moduleQueue)
 		block();
-	else
+	else if (synchronously)
 		dispatch_sync(moduleQueue, block);
+	else
+		dispatch_async(moduleQueue, block);
+	
+}
+- (void)removeDelegate:(id)delegate delegateQueue:(dispatch_queue_t)delegateQueue
+{
+	// Synchronous operation (common-case default)
+	
+	[self removeDelegate:delegate delegateQueue:delegateQueue synchronously:YES];
 }
 
 - (void)removeDelegate:(id)delegate
 {
-	// Synchronous operation
-	// 
-	// Delegate remove MUST always be synchronous.
-	
-	dispatch_block_t block = ^{
-		[multicastDelegate removeDelegate:delegate];
-	};
+	// Synchronous operation (common-case default)
 
-	if (dispatch_get_current_queue() == moduleQueue)
-		block();
-	else
-		dispatch_sync(moduleQueue, block);
+	[self removeDelegate:delegate delegateQueue:NULL synchronously:YES];
 }
 
 - (NSString *)moduleName

Core/XMPPStream.m

@@ -3923,7 +3923,12 @@ - (void)unregisterModule:(XMPPModule *)module
 
 		while ([autoDelegatesEnumerator getNextDelegate:&delegate delegateQueue:&delegateQueue])
 		{
-			[module removeDelegate:delegate delegateQueue:delegateQueue];
+			// The module itself has dispatch_sync'd in order to invoke its deactivate method,
+			// which has in turn invoked this method. If we call back into the module,
+			// and have it dispatch_sync again, we're going to get a deadlock.
+			// So we must remove the delegate(s) asynchronously.
+			
+			[module removeDelegate:delegate delegateQueue:delegateQueue synchronously:NO];
 		}
 
 		// Unregister modules