Merge pull request #2061 from morganchen12/gulchangelist

Invalidate non-background url sessions

Author: morganchen12

Example/tvOSSample/tvOSSample/AuthViewController.swift

@@ -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).

Example/tvOSSample/tvOSSample/EmailLoginViewController.swift

@@ -21,7 +21,6 @@ protocol EmailLoginDelegate {
 }
 
 class EmailLoginViewController: UIViewController {
-
   // MARK: - Public Properties
 
   var delegate: EmailLoginDelegate?
@@ -65,8 +64,7 @@ class EmailLoginViewController: UIViewController {
 
   // MARK: - View Controller Lifecycle
 
-  override func viewDidLoad() {
-  }
+  override func viewDidLoad() {}
 
   // MARK: - Helper Methods
 

GoogleUtilities/CHANGELOG.md

@@ -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

GoogleUtilities/Network/GULNetworkURLSession.m

@@ -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;
@@ -45,6 +50,9 @@ @implementation GULNetworkURLSession {
 
   /// The current request.
   NSURLRequest *_request;
+
+  /// The current NSURLSession.
+  NSURLSession *__weak _Nullable _URLSession;
 }
 
 #pragma mark - Init
@@ -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];
@@ -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]];
@@ -157,6 +164,8 @@ - (NSString *)sessionIDFromAsyncPOSTRequest:(NSURLRequest *)request
     return nil;
   }
 
+  _URLSession = session;
+
   // Save the session into memory.
   [[self class] setSessionInFetcherMap:self forSessionID:_sessionID];
 
@@ -199,6 +208,8 @@ - (NSString *)sessionIDFromAsyncGETRequest:(NSURLRequest *)request
     return nil;
   }
 
+  _URLSession = session;
+
   // Save the session into memory.
   [[self class] setSessionInFetcherMap:self forSessionID:_sessionID];
 
@@ -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
@@ -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];

GoogleUtilities/Network/Private/GULNetworkURLSession.h

@@ -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