Fix NSE Leak (#2022)

ElementX.xcodeproj/project.pbxproj

@@ -6592,7 +6592,7 @@
 			repositoryURL = "https://github.com/matrix-org/matrix-rust-components-swift";
 			requirement = {
 				kind = exactVersion;
-				version = "0.0.2-october23";
+				version = "0.0.1-november23";
 			};
 		};
 		821C67C9A7F8CC3FD41B28B4 /* XCRemoteSwiftPackageReference "emojibase-bindings" */ = {

ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

@@ -130,8 +130,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/matrix-org/matrix-rust-components-swift",
       "state" : {
-        "revision" : "48f7062bd94debe766820a28c77f40a6e51900c3",
+        "revision" : "29870facdcf257e9cd79ee0eacd52b7425b92736",
-        "version" : "0.0.2-october23"
+        "version" : "0.0.1-november23"
       }
     },
     {

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.1.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.1.1 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 
 // swiftlint:disable all

ElementX/Sources/Mocks/Generated/SDKGeneratedMocks.swift

@@ -575,13 +575,18 @@ class SDKClientMock: SDKClientProtocol {
     }
     public var setDelegateDelegateReceivedDelegate: ClientDelegate?
     public var setDelegateDelegateReceivedInvocations: [ClientDelegate?] = []
-    public var setDelegateDelegateClosure: ((ClientDelegate?) -> Void)?
+    public var setDelegateDelegateReturnValue: TaskHandle?
+    public var setDelegateDelegateClosure: ((ClientDelegate?) -> TaskHandle?)?
 
-    public func setDelegate(delegate: ClientDelegate?) {
+    public func setDelegate(delegate: ClientDelegate?) -> TaskHandle? {
         setDelegateDelegateCallsCount += 1
         setDelegateDelegateReceivedDelegate = delegate
         setDelegateDelegateReceivedInvocations.append(delegate)
-        setDelegateDelegateClosure?(delegate)
+        if let setDelegateDelegateClosure = setDelegateDelegateClosure {
+            return setDelegateDelegateClosure(delegate)
+        } else {
+            return setDelegateDelegateReturnValue
+        }
     }
     //MARK: - setDisplayName
 

ElementX/Sources/Services/Client/ClientProxy.swift

@@ -36,6 +36,8 @@ class ClientProxy: ClientProxyProtocol {
     private var syncService: SyncService?
     private var syncServiceStateUpdateTaskHandle: TaskHandle?
     
+    private var delegateHandle: TaskHandle?
+    
     // These following summary providers both operate on the same allRooms() list but
     // can apply their own filtering and pagination
     private(set) var roomSummaryProvider: RoomSummaryProviderProtocol?
@@ -69,8 +71,10 @@ class ClientProxy: ClientProxyProtocol {
     private var hasEncounteredAuthError = false
     
     deinit {
-        client.setDelegate(delegate: nil)
+        stopSync { [delegateHandle] in
-        stopSync()
+            // The delegate handle needs to be cancelled always after the sync stops
+            delegateHandle?.cancel()
+        }
     }
     
     let callbacks = PassthroughSubject<ClientProxyCallback, Never>()
@@ -98,7 +102,7 @@ class ClientProxy: ClientProxyProtocol {
         
         secureBackupController = SecureBackupController(encryption: client.encryption())
 
-        client.setDelegate(delegate: ClientDelegateWrapper { [weak self] isSoftLogout in
+        delegateHandle = client.setDelegate(delegate: ClientDelegateWrapper { [weak self] isSoftLogout in
             self?.hasEncounteredAuthError = true
             self?.callbacks.send(.receivedAuthError(isSoftLogout: isSoftLogout))
         })
@@ -168,10 +172,17 @@ class ClientProxy: ClientProxyProtocol {
     }
     
     func stopSync() {
+        stopSync(completion: nil)
+    }
+    
+    private func stopSync(completion: (() -> Void)?) {
         MXLog.info("Stopping sync")
         
         Task {
             do {
+                defer {
+                    completion?()
+                }
                 try await syncService?.stop()
             } catch {
                 MXLog.error("Failed stopping the sync service with error: \(error)")

ElementX/Sources/Services/ElementCall/ElementCallWidgetDriver.swift

@@ -67,7 +67,8 @@ class ElementCallWidgetDriver: WidgetCapabilitiesProvider, ElementCallWidgetDriv
                                                                                  skipLobby: true,
                                                                                  confineToRoom: true,
                                                                                  font: nil,
-                                                                                 analyticsId: nil)) else {
+                                                                                 analyticsId: nil,
+                                                                                 encryption: .unencrypted)) else {
             return .failure(.failedBuildingWidgetSettings)
         }
         

ElementX/Sources/Services/Media/Provider/MediaFileHandleProxy.swift

@@ -59,9 +59,13 @@ extension MediaFileHandleProxy: Hashable {
 /// An unmanaged file handle that can be created direct from a URL.
 ///
 /// This type allows for mocking but doesn't provide the automatic clean-up mechanism provided by the SDK.
-private struct UnmanagedMediaFileHandle: MediaFileHandleProtocol {
+private class UnmanagedMediaFileHandle: MediaFileHandleProtocol {
     let url: URL
     
+    init(url: URL) {
+        self.url = url
+    }
+    
     func path() -> String {
         url.path()
     }

NSE/Sources/NotificationServiceExtension.swift

@@ -18,15 +18,38 @@ import Intents
 import MatrixRustSDK
 import UserNotifications
 
+// The lifecycle of the NSE looks something like the following:
+//  1)  App receives notification
+//  2)  System creates an instance of the extension class
+//      and calls `didReceive` in the background
+//  3)  Extension processes messages / displays whatever
+//      notifications it needs to
+//  4)  Extension notifies its work is complete by calling
+//      the contentHandler
+//  5)  If the extension takes too long to perform its work
+//      (more than 30s), it will be notified and immediately
+//      terminated
+//
+// Note that the NSE does *not* always spawn a new process to
+// handle a new notification and will also try and process notifications
+// in parallel. `didReceive` could be called twice for the same process,
+// but it will always be called on different threads. It may or may not be
+// called on the same instance of `NotificationService` as a previous
+// notification.
+//
+// We keep a global `environment` singleton to ensure that our app context,
+// database, logging, etc. are only ever setup once per *process*
+
+private let settings = NSESettings()
+private let notificationContentBuilder = NotificationContentBuilder(messageEventStringBuilder: RoomMessageEventStringBuilder(attributedStringBuilder: AttributedStringBuilder(permalinkBaseURL: .homeDirectory,
+                                                                                                                                                                              mentionBuilder: PlainMentionBuilder())))
+private let keychainController = KeychainController(service: .sessions,
+                                                    accessGroup: InfoPlistReader.main.keychainAccessGroupIdentifier)
+private var userSessions = [String: NSEUserSession]()
+
 class NotificationServiceExtension: UNNotificationServiceExtension {
-    private let settings = NSESettings()
-    private let notificationContentBuilder = NotificationContentBuilder(messageEventStringBuilder: RoomMessageEventStringBuilder(attributedStringBuilder: AttributedStringBuilder(permalinkBaseURL: .homeDirectory,
-                                                                                                                                                                                  mentionBuilder: PlainMentionBuilder())))
-    private lazy var keychainController = KeychainController(service: .sessions,
-                                                             accessGroup: InfoPlistReader.main.keychainAccessGroupIdentifier)
     private var handler: ((UNNotificationContent) -> Void)?
     private var modifiedContent: UNMutableNotificationContent?
-    private var userSession: NSEUserSession?
 
     override func didReceive(_ request: UNNotificationRequest,
                              withContentHandler contentHandler: @escaping (UNNotificationContent) -> Void) {
@@ -75,8 +98,13 @@ class NotificationServiceExtension: UNNotificationServiceExtension {
         MXLog.info("\(tag) run with roomId: \(roomId), eventId: \(eventId)")
 
         do {
-            let userSession = try NSEUserSession(credentials: credentials, clientSessionDelegate: keychainController)
+            let userSession: NSEUserSession
-            self.userSession = userSession
+            if let existingSession = userSessions[credentials.userID] {
+                userSession = existingSession
+            } else {
+                userSession = try NSEUserSession(credentials: credentials, clientSessionDelegate: keychainController)
+                userSessions[credentials.userID] = userSession
+            }
             
             guard let itemProxy = await userSession.notificationItemProxy(roomID: roomId, eventID: eventId) else {
                 MXLog.info("\(tag) no notification for the event, discard")

NSE/Sources/Other/NSEUserSession.swift

@@ -24,6 +24,7 @@ final class NSEUserSession {
     private(set) lazy var mediaProvider: MediaProviderProtocol = MediaProvider(mediaLoader: MediaLoader(client: baseClient),
                                                                                imageCache: .onlyOnDisk,
                                                                                backgroundTaskService: nil)
+    private let delegateHandle: TaskHandle?
 
     init(credentials: KeychainCredentials, clientSessionDelegate: ClientSessionDelegate) throws {
         userID = credentials.userID
@@ -35,7 +36,7 @@ final class NSEUserSession {
                                            sessionDelegate: clientSessionDelegate)
             .build()
         
-        baseClient.setDelegate(delegate: ClientDelegateWrapper())
+        delegateHandle = baseClient.setDelegate(delegate: ClientDelegateWrapper())
         try baseClient.restoreSession(session: credentials.restorationToken.session)
         
         notificationClient = try baseClient
@@ -62,6 +63,10 @@ final class NSEUserSession {
             }
         }
     }
+    
+    deinit {
+        delegateHandle?.cancel()
+    }
 }
 
 private class ClientDelegateWrapper: ClientDelegate {

changelog.d/1923.bugfix

@@ -0,0 +1 @@
+Fixed a memory leak that made the NSE crash.

project.yml

@@ -45,7 +45,7 @@ packages:
   # Element/Matrix dependencies
   MatrixRustSDK:
     url: https://github.com/matrix-org/matrix-rust-components-swift
-    exactVersion: 0.0.2-october23
+    exactVersion: 0.0.1-november23
     # path: ../matrix-rust-sdk
   Compound:
     url: https://github.com/vector-im/compound-ios