Skip to content
Browse files

Fixing potential deadlock situation involving modules and autoDelegates.

  • Loading branch information...
1 parent 86f7a8f commit 35ab9a847af5ae35f04384808994a2eb326bfd4e @robbiehanson committed Jul 29, 2012
Showing with 32 additions and 19 deletions.
  1. +13 −1 Core/XMPPInternal.h
  2. +13 −17 Core/XMPPModule.m
  3. +6 −1 Core/XMPPStream.m
View
14 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
View
30 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
View
7 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

0 comments on commit 35ab9a8

Please sign in to comment.
Something went wrong with that request. Please try again.