Skip to content

Memory leak fix and cancel connection timeout timer in scheduleCleanup. #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Memory leak fix and cancel connection timeout timer in scheduleCleanup. #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f50e05
Memory leak fix and cancel connection timeout timer in scheduleCleanup.
Jul 20, 2017
a68d756
Adds missing SRIOConsumerPool:clear method.
Jul 20, 2017
a72c597
fix -Warc-repeated-use-of-weak warning.
Jul 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions SocketRocket/Internal/IOConsumer/SRIOConsumerPool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@
unmaskBytes:(BOOL)unmaskBytes;
- (void)returnConsumer:(SRIOConsumer *)consumer;

- (void)clear;

@end
6 changes: 6 additions & 0 deletions SocketRocket/Internal/IOConsumer/SRIOConsumerPool.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,10 @@ - (void)returnConsumer:(SRIOConsumer *)consumer;
}
}

- (void)clear
{
_poolSize = 0;
_bufferedConsumers = nil;
}

@end
45 changes: 33 additions & 12 deletions SocketRocket/SRWebSocket.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,14 @@ + (BOOL)automaticallyNotifiesObserversOfReadyState {
return NO;
}

-(void)_onTimeout
{
if (self.readyState == SR_CONNECTING) {
NSError *error = SRErrorWithDomainCodeDescription(NSURLErrorDomain, NSURLErrorTimedOut, @"Timed out connecting to server.");
[self _failWithError:error];
}
}

///--------------------------------------
#pragma mark - Open / Close
///--------------------------------------
Expand All @@ -307,13 +315,7 @@ - (void)open
_selfRetain = self;

if (_urlRequest.timeoutInterval > 0) {
dispatch_time_t popTime = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(_urlRequest.timeoutInterval * NSEC_PER_SEC));
dispatch_after(popTime, dispatch_get_main_queue(), ^{
if (self.readyState == SR_CONNECTING) {
NSError *error = SRErrorWithDomainCodeDescription(NSURLErrorDomain, NSURLErrorTimedOut, @"Timed out connecting to server.");
[self _failWithError:error];
}
});
[self performSelector:@selector(_onTimeout) withObject:nil afterDelay:_urlRequest.timeoutInterval];
}

_proxyConnect = [[SRProxyConnect alloc] initWithURL:_url];
Expand Down Expand Up @@ -419,14 +421,23 @@ - (void)_readHTTPHeader;
_receivedHTTPHeaders = CFHTTPMessageCreateEmpty(NULL, NO);
}

// Uses weak self object in the block, otherwise Consumers will retain SRWebSocket instance,
// and SRWebSocket instance also hold consumers, cycle reference will occur.
__weak typeof(self) wself = self;

[self _readUntilHeaderCompleteWithCallback:^(SRWebSocket *socket, NSData *data) {
CFHTTPMessageAppendBytes(_receivedHTTPHeaders, (const UInt8 *)data.bytes, data.length);

if (CFHTTPMessageIsHeaderComplete(_receivedHTTPHeaders)) {
SRDebugLog(@"Finished reading headers %@", CFBridgingRelease(CFHTTPMessageCopyAllHeaderFields(_receivedHTTPHeaders)));
[self _HTTPHeadersDidFinish];
__strong typeof(wself) sself = wself;
if (sself == nil)
return;

CFHTTPMessageAppendBytes(sself.receivedHTTPHeaders, (const UInt8 *)data.bytes, data.length);

if (CFHTTPMessageIsHeaderComplete(sself.receivedHTTPHeaders)) {
SRDebugLog(@"Finished reading headers %@", CFBridgingRelease(CFHTTPMessageCopyAllHeaderFields(sself.receivedHTTPHeaders)));
[sself _HTTPHeadersDidFinish];
} else {
[self _readHTTPHeader];
[sself _readHTTPHeader];
}
}];
}
Expand Down Expand Up @@ -1127,6 +1138,16 @@ - (void)_scheduleCleanup

_cleanupScheduled = YES;

// _consumers retain SRWebSocket instance by block copy, if there are consumers here, clear them.
[_consumers removeAllObjects];
[_consumerPool clear];

// Cancel the timer which retains SRWebSocket instance.
// If we don't cancel the timer, the 'dealloc' method will be invoked only after the time (default: 60s) have come, which may cause memory increase.
dispatch_async(dispatch_get_main_queue(), ^(){
[NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(_onTimeout) object:nil];
});

// Cleanup NSStream delegate's in the same RunLoop used by the streams themselves:
// This way we'll prevent race conditions between handleEvent and SRWebsocket's dealloc
NSTimer *timer = [NSTimer timerWithTimeInterval:(0.0f) target:self selector:@selector(_cleanupSelfReference:) userInfo:nil repeats:NO];
Expand Down