Make close logic thread safe

Before close method was not thread safe. Callers should make sure that they are on right queue. Also fix issue where session not closed when on current queue.
novastone-media · Feb 1, 2018 · 0e21e56 · 0e21e56
1 parent 338ace6
commit 0e21e56
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 21 deletions.
diff --git a/MQTTClient/MQTTClient/MQTTCFSocketTransport.m b/MQTTClient/MQTTClient/MQTTCFSocketTransport.m
@@ -37,20 +37,7 @@ - (instancetype)init {
 }
 
 - (void)dealloc {
-    // https://github.com/novastone-media/MQTT-Client-Framework/issues/325
-    // It is probably not the best solution to use deprecated API
-    // but it is bug that happens quite often so it is important to fix it
-    // and if we find better solution we can change it later
-
-    // We need to make sure that we are closing streams on their queue
-    // Otherwise, we end up with race condition where delegate is deallocated
-    // but still used by run loop event
-    if (self.queue != dispatch_get_current_queue()) {
-        dispatch_sync(self.queue, ^{
-            [self.encoder close];
-            [self.decoder close];
-        });
-    }
+    [self close];
 }
 
 - (void)open {
@@ -115,6 +102,24 @@ - (void)open {
 }
 
 - (void)close {
+    // https://github.com/novastone-media/MQTT-Client-Framework/issues/325
+    // It is probably not the best solution to use deprecated API
+    // but it is bug that happens quite often so it is important to fix it
+    // and if we find better solution we can change it later
+
+    // We need to make sure that we are closing streams on their queue
+    // Otherwise, we end up with race condition where delegate is deallocated
+    // but still used by run loop event
+    if (self.queue != dispatch_get_current_queue()) {
+        dispatch_sync(self.queue, ^{
+            [self internalClose];
+        });
+    } else {
+        [self internalClose];
+    }
+}
+
+- (void)internalClose {
     DDLogVerbose(@"[MQTTCFSocketTransport] close");
     self.state = MQTTTransportClosing;
 

diff --git a/MQTTClient/MQTTClient/MQTTDecoder.m b/MQTTClient/MQTTClient/MQTTDecoder.m
@@ -47,6 +47,16 @@ - (void)open {
     self.state = MQTTDecoderStateDecodingHeader;
 }
 
+- (void)internalClose {
+    if (self.streams) {
+        for (NSInputStream *stream in self.streams) {
+            [stream close];
+            [stream setDelegate:nil];
+        }
+        [self.streams removeAllObjects];
+    }
+}
+
 - (void)close {
     // https://github.com/novastone-media/MQTT-Client-Framework/issues/325
     // It is probably not the best solution to use deprecated API
@@ -58,14 +68,10 @@ - (void)close {
     // but still used by run loop event
     if (self.queue != dispatch_get_current_queue()) {
         dispatch_sync(self.queue, ^{
-            if (self.streams) {
-                for (NSInputStream *stream in self.streams) {
-                    [stream close];
-                    [stream setDelegate:nil];
-                }
-                [self.streams removeAllObjects];
-            }
+            [self internalClose];
         });
+    } else {
+        [self internalClose];
     }
 }