Skip to content

Commit aea16a4

Browse files
committed
added more safety to IoTStreamThread
1 parent 3a75df4 commit aea16a4

File tree

1 file changed

+27
-63
lines changed

1 file changed

+27
-63
lines changed

AWSIoT/Internal/AWSIoTStreamThread.m

Lines changed: 27 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -46,27 +46,6 @@ - (void)dealloc {
4646
[self performMinimalCleanup];
4747
}
4848

49-
/**
50-
* MINIMAL CLEANUP - Used during dealloc for crash safety
51-
*
52-
* What it does:
53-
* - Stops timer to prevent callbacks
54-
* - Closes all streams and session directly with @synchronized for thread safety
55-
* - Clears delegates and blocks
56-
* - NO runloop operations (removeFromRunLoop)
57-
* - NO conditional logic based on shouldDisconnect
58-
*
59-
* Why minimal:
60-
* - Safe during object deallocation
61-
* - Avoids method calls that could crash
62-
* - Simple operations only
63-
* - Prevents use-after-free crashes
64-
*
65-
* Thread Safety:
66-
* - Uses @synchronized per stream object (customer's proven approach)
67-
* - Combined with atomic properties for comprehensive protection
68-
* - Double-checks stream existence within synchronized blocks
69-
*/
7049
- (void)performMinimalCleanup {
7150
if (_didCleanUp) {
7251
return;
@@ -159,16 +138,13 @@ - (void)main {
159138
[self.session connectToInputStream:self.decoderInputStream
160139
outputStream:self.encoderOutputStream];
161140

162-
// Add protection against runloop corruption from multiple threads
163-
@try {
164-
while ([self shouldContinueRunning]) {
165-
//This will continue run until the thread is cancelled
166-
//Run one cycle of the runloop. This will return after a input source event or timer event is processed
167-
[self.runLoopForStreamsThread runMode:NSDefaultRunLoopMode
168-
beforeDate:[NSDate dateWithTimeIntervalSinceNow:self.defaultRunLoopTimeInterval]];
169-
}
170-
} @catch (NSException *exception) {
171-
AWSDDLogError(@"Exception in runloop execution: %@", exception);
141+
// Capture values locally to prevent crashes if self is deallocated
142+
NSRunLoop *runLoop = self.runLoopForStreamsThread;
143+
NSTimeInterval interval = self.defaultRunLoopTimeInterval;
144+
145+
while ([self shouldContinueRunning] && runLoop) {
146+
[runLoop runMode:NSDefaultRunLoopMode
147+
beforeDate:[NSDate dateWithTimeIntervalSinceNow:interval]];
172148
}
173149

174150
[self cleanUp];
@@ -181,7 +157,11 @@ - (BOOL)shouldContinueRunning {
181157
return NO;
182158
}
183159

184-
return self.isRunning && !self.isCancelled && self.defaultRunLoopTimer != nil;
160+
if (!self.runLoopForStreamsThread || !self.defaultRunLoopTimer) {
161+
return NO;
162+
}
163+
164+
return self.isRunning && !self.isCancelled;
185165
}
186166

187167
- (void)cancel {
@@ -191,6 +171,12 @@ - (void)cancel {
191171
return;
192172
}
193173

174+
// Invalidate timer immediately to break retain cycle
175+
if (self.defaultRunLoopTimer) {
176+
[self.defaultRunLoopTimer invalidate];
177+
self.defaultRunLoopTimer = nil;
178+
}
179+
194180
// Atomic property, no synchronization needed
195181
self.isRunning = NO;
196182
[super cancel];
@@ -203,39 +189,18 @@ - (void)cancelAndDisconnect:(BOOL)shouldDisconnect {
203189
return;
204190
}
205191

192+
// Invalidate timer immediately to break retain cycle
193+
if (self.defaultRunLoopTimer) {
194+
[self.defaultRunLoopTimer invalidate];
195+
self.defaultRunLoopTimer = nil;
196+
}
197+
206198
// Set flags and cancel - properties are atomic
207199
self.shouldDisconnect = shouldDisconnect;
208200
self.isRunning = NO;
209201
[super cancel];
210202
}
211203

212-
/**
213-
* FULL CLEANUP - Called during normal thread shutdown
214-
*
215-
* When used:
216-
* - Thread finishes main() execution normally
217-
* - Thread is cancelled via cancel() or cancelAndDisconnect()
218-
* - App is terminating or going to background
219-
*
220-
* What it does:
221-
* - Stops timer to prevent callbacks
222-
* - Conditionally closes streams based on shouldDisconnect flag
223-
* - Properly removes streams from runloop before closing with @synchronized
224-
* - Follows original disconnect logic
225-
* - Handles onStop callback on main thread
226-
*
227-
* Why full cleanup is safe here:
228-
* - Object is still valid and not being deallocated
229-
* - Safe to perform runloop operations
230-
* - Need to respect shouldDisconnect flag for proper behavior
231-
* - Can safely dispatch onStop callback to main thread
232-
*
233-
* Thread Safety Strategy:
234-
* - @synchronized blocks prevent race conditions during stream operations
235-
* - Atomic properties ensure consistent state across threads
236-
* - Double-checks within sync blocks prevent operating on closed streams
237-
* - Combines customer's surgical approach with our comprehensive coverage
238-
*/
239204
- (void)cleanUp {
240205
if (self.didCleanUp) {
241206
return;
@@ -248,6 +213,9 @@ - (void)cleanUp {
248213
self.defaultRunLoopTimer = nil;
249214
}
250215

216+
// Clear runloop reference
217+
self.runLoopForStreamsThread = nil;
218+
251219
// Conditional cleanup based on shouldDisconnect flag
252220
if (self.shouldDisconnect) {
253221
// Close session first to cleanly terminate MQTT connection
@@ -260,10 +228,6 @@ - (void)cleanUp {
260228
@synchronized(self.outputStream) {
261229
if (self.outputStream) {
262230
self.outputStream.delegate = nil;
263-
// Safe to do runloop operations during normal cleanup
264-
if (self.runLoopForStreamsThread) {
265-
[self.outputStream removeFromRunLoop:self.runLoopForStreamsThread forMode:NSDefaultRunLoopMode];
266-
}
267231
[self.outputStream close];
268232
self.outputStream = nil;
269233
}

0 commit comments

Comments
 (0)