Skip to content

Invalidate non-background url sessions #2061

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.

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

Merged
merged 4 commits into from
Nov 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Example/tvOSSample/tvOSSample/AuthViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import UIKit
import FirebaseAuth

class AuthViewController: UIViewController {

// MARK: - User Interface

/// A stackview containing all of the buttons to providers (Email, OAuth, etc).
Expand Down
4 changes: 1 addition & 3 deletions Example/tvOSSample/tvOSSample/EmailLoginViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ protocol EmailLoginDelegate {
}

class EmailLoginViewController: UIViewController {

// MARK: - Public Properties

var delegate: EmailLoginDelegate?
Expand Down Expand Up @@ -65,8 +64,7 @@ class EmailLoginViewController: UIViewController {

// MARK: - View Controller Lifecycle

override func viewDidLoad() {
}
override func viewDidLoad() {}

// MARK: - Helper Methods

Expand Down
2 changes: 2 additions & 0 deletions GoogleUtilities/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# Unreleased
- Fixed an issue where GoogleUtilities would leak non-background URL sessions.
(#2061)

# 5.3.4
- Fixed a crash caused by unprotected access to sessions in
Expand Down
46 changes: 28 additions & 18 deletions GoogleUtilities/Network/GULNetworkURLSession.m
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
#import "Private/GULNetworkConstants.h"
#import "Private/GULNetworkMessageCode.h"

@interface GULNetworkURLSession () <NSURLSessionDelegate,
NSURLSessionTaskDelegate,
NSURLSessionDownloadDelegate>
@end

@implementation GULNetworkURLSession {
/// The handler to be called when the request completes or error has occurs.
GULNetworkURLSessionCompletionHandler _completionHandler;
Expand All @@ -45,6 +50,9 @@ @implementation GULNetworkURLSession {

/// The current request.
NSURLRequest *_request;

/// The current NSURLSession.
NSURLSession *__weak _Nullable _URLSession;
}

#pragma mark - Init
Expand Down Expand Up @@ -128,7 +136,7 @@ - (NSString *)sessionIDFromAsyncPOSTRequest:(NSURLRequest *)request

if (didWriteFile) {
// Exclude this file from backing up to iTunes. There are conflicting reports that excluding
// directory from backing up does not excluding files of that directory from backing up.
// directory from backing up does not exclude files of that directory from backing up.
[self excludeFromBackupForURL:_uploadingFileURL];

_sessionConfig = [self backgroundSessionConfigWithSessionID:_sessionID];
Expand All @@ -141,7 +149,6 @@ - (NSString *)sessionIDFromAsyncPOSTRequest:(NSURLRequest *)request
// If we cannot write to file, just send it in the foreground.
_sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration];
[self populateSessionConfig:_sessionConfig withRequest:request];
_sessionConfig.URLCache = nil;
session = [NSURLSession sessionWithConfiguration:_sessionConfig
delegate:self
delegateQueue:[NSOperationQueue mainQueue]];
Expand All @@ -157,6 +164,8 @@ - (NSString *)sessionIDFromAsyncPOSTRequest:(NSURLRequest *)request
return nil;
}

_URLSession = session;

// Save the session into memory.
[[self class] setSessionInFetcherMap:self forSessionID:_sessionID];

Expand Down Expand Up @@ -199,6 +208,8 @@ - (NSString *)sessionIDFromAsyncGETRequest:(NSURLRequest *)request
return nil;
}

_URLSession = session;

// Save the session into memory.
[[self class] setSessionInFetcherMap:self forSessionID:_sessionID];

Expand Down Expand Up @@ -283,16 +294,15 @@ - (void)URLSession:(NSURLSession *)session
[self maybeRemoveTempFilesAtURL:_networkDirectoryURL
expiringTime:kGULNetworkTempFolderExpireTime];

// Invalidate the session only if it's owned by this class.
NSString *sessionID = session.configuration.identifier;
if ([sessionID hasPrefix:kGULNetworkBackgroundSessionConfigIDPrefix]) {
[session finishTasksAndInvalidate];
// This is called without checking the sessionID here since non-background sessions
// won't have an ID.
[session finishTasksAndInvalidate];

// Explicitly remove the session so it won't be reused. The weak map table should
// remove the session on deallocation, but dealloc may not happen immediately after
// calling `finishTasksAndInvalidate`.
[[self class] setSessionInFetcherMap:nil forSessionID:sessionID];
}
// Explicitly remove the session so it won't be reused. The weak map table should
// remove the session on deallocation, but dealloc may not happen immediately after
// calling `finishTasksAndInvalidate`.
NSString *sessionID = session.configuration.identifier;
[[self class] setSessionInFetcherMap:nil forSessionID:sessionID];
}

- (void)URLSession:(NSURLSession *)session
Expand Down Expand Up @@ -677,13 +687,13 @@ + (void)setSessionInFetcherMap:(GULNetworkURLSession *)session forSessionID:(NSS
GULNetworkURLSession *existingSession =
[[[self class] sessionIDToFetcherMap] objectForKey:sessionID];
if (existingSession) {
// Invalidating doesn't seem like the right thing to do here since it may cancel an active
// background transfer if the background session is handling multiple requests. The old
// session will be dropped from the map table, but still complete its request.
NSString *message = [NSString stringWithFormat:@"Discarding session: %@", existingSession];
[existingSession->_loggerDelegate GULNetwork_logWithLevel:kGULNetworkLogLevelInfo
messageCode:kGULNetworkMessageCodeURLSession019
message:message];
if (session) {
NSString *message = [NSString stringWithFormat:@"Discarding session: %@", existingSession];
[existingSession->_loggerDelegate GULNetwork_logWithLevel:kGULNetworkLogLevelInfo
messageCode:kGULNetworkMessageCodeURLSession019
message:message];
}
[existingSession->_URLSession finishTasksAndInvalidate];
}
if (session) {
[[[self class] sessionIDToFetcherMap] setObject:session forKey:sessionID];
Expand Down
5 changes: 2 additions & 3 deletions GoogleUtilities/Network/Private/GULNetworkURLSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ typedef void (^GULNetworkURLSessionCompletionHandler)(NSHTTPURLResponse *respons
typedef void (^GULNetworkSystemCompletionHandler)(void);

/// The protocol that uses NSURLSession for iOS >= 7.0 to handle requests and responses.
@interface GULNetworkURLSession
: NSObject <NSURLSessionDelegate, NSURLSessionTaskDelegate, NSURLSessionDownloadDelegate>
@interface GULNetworkURLSession : NSObject

/// Indicates whether the background network is enabled. Default value is NO.
@property(nonatomic, getter=isBackgroundNetworkEnabled) BOOL backgroundNetworkEnabled;

/// The logger delegate to log message, errors or warnings that occur during the network operations.
@property(nonatomic, weak) id<GULNetworkLoggerDelegate> loggerDelegate;
@property(nonatomic, weak, nullable) id<GULNetworkLoggerDelegate> loggerDelegate;

/// Calls the system provided completion handler after the background session is finished.
+ (void)handleEventsForBackgroundURLSessionID:(NSString *)sessionID
Expand Down