From 8d069fb74c3989a0f386b02af6d7d54ceef87e32 Mon Sep 17 00:00:00 2001 From: Doug Date: Wed, 3 Sep 2025 16:48:55 +0100 Subject: [PATCH] Workaround: Hold a weak reference to the SDK's Client in MediaLoader so it can be released when clearing the cache. --- ElementX/Sources/Mocks/ClientProxyMock.swift | 8 +- .../Mocks/Generated/GeneratedMocks.swift | 227 +----------------- .../Sources/Services/Client/ClientProxy.swift | 16 +- .../Services/Client/ClientProxyProtocol.swift | 4 +- .../Services/Media/Provider/MediaLoader.swift | 23 +- .../Media/Provider/MediaLoaderProtocol.swift | 4 + .../UserSession/UserSessionStore.swift | 2 +- 7 files changed, 37 insertions(+), 247 deletions(-) diff --git a/ElementX/Sources/Mocks/ClientProxyMock.swift b/ElementX/Sources/Mocks/ClientProxyMock.swift index 09c3b88d7..d209cc70f 100644 --- a/ElementX/Sources/Mocks/ClientProxyMock.swift +++ b/ElementX/Sources/Mocks/ClientProxyMock.swift @@ -80,9 +80,11 @@ extension ClientProxyMock { recentlyVisitedRoomsReturnValue = .success([]) recentConversationCounterpartsReturnValue = [] - loadMediaContentForSourceThrowableError = ClientProxyError.sdkError(ClientProxyMockError.generic) - loadMediaThumbnailForSourceWidthHeightThrowableError = ClientProxyError.sdkError(ClientProxyMockError.generic) - loadMediaFileForSourceFilenameThrowableError = ClientProxyError.sdkError(ClientProxyMockError.generic) + let mediaLoader = MediaLoaderMock() + mediaLoader.loadMediaContentForSourceThrowableError = ClientProxyError.sdkError(ClientProxyMockError.generic) + mediaLoader.loadMediaThumbnailForSourceWidthHeightThrowableError = ClientProxyError.sdkError(ClientProxyMockError.generic) + mediaLoader.loadMediaFileForSourceFilenameThrowableError = ClientProxyError.sdkError(ClientProxyMockError.generic) + self.mediaLoader = mediaLoader secureBackupController = SecureBackupControllerMock(.init(recoveryState: configuration.recoveryState)) resetIdentityReturnValue = .success(IdentityResetHandleSDKMock(.init())) diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index ec760a56b..a0b672c21 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -2125,6 +2125,11 @@ class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable { } var underlyingHideInviteAvatarsPublisher: CurrentValuePublisher! var pusherNotificationClientIdentifier: String? + var mediaLoader: MediaLoaderProtocol { + get { return underlyingMediaLoader } + set(value) { underlyingMediaLoader = value } + } + var underlyingMediaLoader: MediaLoaderProtocol! var roomSummaryProvider: RoomSummaryProviderProtocol { get { return underlyingRoomSummaryProvider } set(value) { underlyingRoomSummaryProvider = value } @@ -5199,228 +5204,6 @@ class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable { return setHideInviteAvatarsReturnValue } } - //MARK: - loadMediaContentForSource - - var loadMediaContentForSourceThrowableError: Error? - var loadMediaContentForSourceUnderlyingCallsCount = 0 - var loadMediaContentForSourceCallsCount: Int { - get { - if Thread.isMainThread { - return loadMediaContentForSourceUnderlyingCallsCount - } else { - var returnValue: Int? = nil - DispatchQueue.main.sync { - returnValue = loadMediaContentForSourceUnderlyingCallsCount - } - - return returnValue! - } - } - set { - if Thread.isMainThread { - loadMediaContentForSourceUnderlyingCallsCount = newValue - } else { - DispatchQueue.main.sync { - loadMediaContentForSourceUnderlyingCallsCount = newValue - } - } - } - } - var loadMediaContentForSourceCalled: Bool { - return loadMediaContentForSourceCallsCount > 0 - } - var loadMediaContentForSourceReceivedSource: MediaSourceProxy? - var loadMediaContentForSourceReceivedInvocations: [MediaSourceProxy] = [] - - var loadMediaContentForSourceUnderlyingReturnValue: Data! - var loadMediaContentForSourceReturnValue: Data! { - get { - if Thread.isMainThread { - return loadMediaContentForSourceUnderlyingReturnValue - } else { - var returnValue: Data? = nil - DispatchQueue.main.sync { - returnValue = loadMediaContentForSourceUnderlyingReturnValue - } - - return returnValue! - } - } - set { - if Thread.isMainThread { - loadMediaContentForSourceUnderlyingReturnValue = newValue - } else { - DispatchQueue.main.sync { - loadMediaContentForSourceUnderlyingReturnValue = newValue - } - } - } - } - var loadMediaContentForSourceClosure: ((MediaSourceProxy) async throws -> Data)? - - func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data { - if let error = loadMediaContentForSourceThrowableError { - throw error - } - loadMediaContentForSourceCallsCount += 1 - loadMediaContentForSourceReceivedSource = source - DispatchQueue.main.async { - self.loadMediaContentForSourceReceivedInvocations.append(source) - } - if let loadMediaContentForSourceClosure = loadMediaContentForSourceClosure { - return try await loadMediaContentForSourceClosure(source) - } else { - return loadMediaContentForSourceReturnValue - } - } - //MARK: - loadMediaThumbnailForSource - - var loadMediaThumbnailForSourceWidthHeightThrowableError: Error? - var loadMediaThumbnailForSourceWidthHeightUnderlyingCallsCount = 0 - var loadMediaThumbnailForSourceWidthHeightCallsCount: Int { - get { - if Thread.isMainThread { - return loadMediaThumbnailForSourceWidthHeightUnderlyingCallsCount - } else { - var returnValue: Int? = nil - DispatchQueue.main.sync { - returnValue = loadMediaThumbnailForSourceWidthHeightUnderlyingCallsCount - } - - return returnValue! - } - } - set { - if Thread.isMainThread { - loadMediaThumbnailForSourceWidthHeightUnderlyingCallsCount = newValue - } else { - DispatchQueue.main.sync { - loadMediaThumbnailForSourceWidthHeightUnderlyingCallsCount = newValue - } - } - } - } - var loadMediaThumbnailForSourceWidthHeightCalled: Bool { - return loadMediaThumbnailForSourceWidthHeightCallsCount > 0 - } - var loadMediaThumbnailForSourceWidthHeightReceivedArguments: (source: MediaSourceProxy, width: UInt, height: UInt)? - var loadMediaThumbnailForSourceWidthHeightReceivedInvocations: [(source: MediaSourceProxy, width: UInt, height: UInt)] = [] - - var loadMediaThumbnailForSourceWidthHeightUnderlyingReturnValue: Data! - var loadMediaThumbnailForSourceWidthHeightReturnValue: Data! { - get { - if Thread.isMainThread { - return loadMediaThumbnailForSourceWidthHeightUnderlyingReturnValue - } else { - var returnValue: Data? = nil - DispatchQueue.main.sync { - returnValue = loadMediaThumbnailForSourceWidthHeightUnderlyingReturnValue - } - - return returnValue! - } - } - set { - if Thread.isMainThread { - loadMediaThumbnailForSourceWidthHeightUnderlyingReturnValue = newValue - } else { - DispatchQueue.main.sync { - loadMediaThumbnailForSourceWidthHeightUnderlyingReturnValue = newValue - } - } - } - } - var loadMediaThumbnailForSourceWidthHeightClosure: ((MediaSourceProxy, UInt, UInt) async throws -> Data)? - - func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data { - if let error = loadMediaThumbnailForSourceWidthHeightThrowableError { - throw error - } - loadMediaThumbnailForSourceWidthHeightCallsCount += 1 - loadMediaThumbnailForSourceWidthHeightReceivedArguments = (source: source, width: width, height: height) - DispatchQueue.main.async { - self.loadMediaThumbnailForSourceWidthHeightReceivedInvocations.append((source: source, width: width, height: height)) - } - if let loadMediaThumbnailForSourceWidthHeightClosure = loadMediaThumbnailForSourceWidthHeightClosure { - return try await loadMediaThumbnailForSourceWidthHeightClosure(source, width, height) - } else { - return loadMediaThumbnailForSourceWidthHeightReturnValue - } - } - //MARK: - loadMediaFileForSource - - var loadMediaFileForSourceFilenameThrowableError: Error? - var loadMediaFileForSourceFilenameUnderlyingCallsCount = 0 - var loadMediaFileForSourceFilenameCallsCount: Int { - get { - if Thread.isMainThread { - return loadMediaFileForSourceFilenameUnderlyingCallsCount - } else { - var returnValue: Int? = nil - DispatchQueue.main.sync { - returnValue = loadMediaFileForSourceFilenameUnderlyingCallsCount - } - - return returnValue! - } - } - set { - if Thread.isMainThread { - loadMediaFileForSourceFilenameUnderlyingCallsCount = newValue - } else { - DispatchQueue.main.sync { - loadMediaFileForSourceFilenameUnderlyingCallsCount = newValue - } - } - } - } - var loadMediaFileForSourceFilenameCalled: Bool { - return loadMediaFileForSourceFilenameCallsCount > 0 - } - var loadMediaFileForSourceFilenameReceivedArguments: (source: MediaSourceProxy, filename: String?)? - var loadMediaFileForSourceFilenameReceivedInvocations: [(source: MediaSourceProxy, filename: String?)] = [] - - var loadMediaFileForSourceFilenameUnderlyingReturnValue: MediaFileHandleProxy! - var loadMediaFileForSourceFilenameReturnValue: MediaFileHandleProxy! { - get { - if Thread.isMainThread { - return loadMediaFileForSourceFilenameUnderlyingReturnValue - } else { - var returnValue: MediaFileHandleProxy? = nil - DispatchQueue.main.sync { - returnValue = loadMediaFileForSourceFilenameUnderlyingReturnValue - } - - return returnValue! - } - } - set { - if Thread.isMainThread { - loadMediaFileForSourceFilenameUnderlyingReturnValue = newValue - } else { - DispatchQueue.main.sync { - loadMediaFileForSourceFilenameUnderlyingReturnValue = newValue - } - } - } - } - var loadMediaFileForSourceFilenameClosure: ((MediaSourceProxy, String?) async throws -> MediaFileHandleProxy)? - - func loadMediaFileForSource(_ source: MediaSourceProxy, filename: String?) async throws -> MediaFileHandleProxy { - if let error = loadMediaFileForSourceFilenameThrowableError { - throw error - } - loadMediaFileForSourceFilenameCallsCount += 1 - loadMediaFileForSourceFilenameReceivedArguments = (source: source, filename: filename) - DispatchQueue.main.async { - self.loadMediaFileForSourceFilenameReceivedInvocations.append((source: source, filename: filename)) - } - if let loadMediaFileForSourceFilenameClosure = loadMediaFileForSourceFilenameClosure { - return try await loadMediaFileForSourceFilenameClosure(source, filename) - } else { - return loadMediaFileForSourceFilenameReturnValue - } - } } class CompletionSuggestionServiceMock: CompletionSuggestionServiceProtocol, @unchecked Sendable { var suggestionsPublisher: AnyPublisher<[SuggestionItem], Never> { diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index dfb79dd39..847100f44 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/ElementX/Sources/Services/Client/ClientProxy.swift @@ -17,7 +17,7 @@ class ClientProxy: ClientProxyProtocol { private let networkMonitor: NetworkMonitorProtocol private let appSettings: AppSettings - private let mediaLoader: MediaLoaderProtocol + let mediaLoader: MediaLoaderProtocol private let clientQueue: DispatchQueue private var roomListService: RoomListService @@ -1113,20 +1113,6 @@ class ClientProxy: ClientProxyProtocol { } } -extension ClientProxy: MediaLoaderProtocol { - func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data { - try await mediaLoader.loadMediaContentForSource(source) - } - - func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data { - try await mediaLoader.loadMediaThumbnailForSource(source, width: width, height: height) - } - - func loadMediaFileForSource(_ source: MediaSourceProxy, filename: String?) async throws -> MediaFileHandleProxy { - try await mediaLoader.loadMediaFileForSource(source, filename: filename) - } -} - private class ClientDelegateWrapper: ClientDelegate { private let authErrorCallback: (Bool) -> Void diff --git a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift index a4bc00157..af6ba8ed4 100644 --- a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift +++ b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift @@ -73,7 +73,7 @@ enum TimelineMediaVisibility: Decodable { } // sourcery: AutoMockable -protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol { +protocol ClientProxyProtocol: AnyObject { var actionsPublisher: AnyPublisher { get } var loadingStatePublisher: CurrentValuePublisher { get } @@ -103,6 +103,8 @@ protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol { var pusherNotificationClientIdentifier: String? { get } + var mediaLoader: MediaLoaderProtocol { get } + var roomSummaryProvider: RoomSummaryProviderProtocol { get } /// Used for listing rooms that shouldn't be affected by the main `roomSummaryProvider` filtering diff --git a/ElementX/Sources/Services/Media/Provider/MediaLoader.swift b/ElementX/Sources/Services/Media/Provider/MediaLoader.swift index 89478fc6c..702ff3735 100644 --- a/ElementX/Sources/Services/Media/Provider/MediaLoader.swift +++ b/ElementX/Sources/Services/Media/Provider/MediaLoader.swift @@ -16,7 +16,17 @@ private final class MediaRequest { } actor MediaLoader: MediaLoaderProtocol { - private let client: ClientProtocol + // Something is holding onto our MediaProvider (and therefore this MediaLoader) which means + // when attempting to clear the caches, the SDK's Client hangs around and we end up with 2. + // I have spent a long time trying to understand what's going on – there's instances of both + // TimelineViewModel.Context and ComposerToolbarViewModel.Context still hanging around and + // a closure captures the media provider from both of those as far as I can tell, but I was + // unable to break the reference. Possibly related to the ElementTextView too. + // + // In lieu of the real fix, lets use a weak reference to the Client here so that it can be + // released and hopefully that will solve our logs files exploding in size when encountering + // a corrupt/missing database file. + private weak var client: ClientProtocol? private var ongoingRequests = [MediaSourceProxy: MediaRequest]() init(client: ClientProtocol) { @@ -24,18 +34,21 @@ actor MediaLoader: MediaLoaderProtocol { } func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data { - try await enqueueLoadMediaRequest(forSource: source) { - try await self.client.getMediaContent(mediaSource: source.underlyingSource) + try await enqueueLoadMediaRequest(forSource: source) { [weak client] in + guard let client else { throw MediaLoaderError.missingClient } + return try await client.getMediaContent(mediaSource: source.underlyingSource) } } func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data { - try await enqueueLoadMediaRequest(forSource: source) { - try await self.client.getMediaThumbnail(mediaSource: source.underlyingSource, width: UInt64(width), height: UInt64(height)) + try await enqueueLoadMediaRequest(forSource: source) { [weak client] in + guard let client else { throw MediaLoaderError.missingClient } + return try await client.getMediaThumbnail(mediaSource: source.underlyingSource, width: UInt64(width), height: UInt64(height)) } } func loadMediaFileForSource(_ source: MediaSourceProxy, filename: String?) async throws -> MediaFileHandleProxy { + guard let client else { throw MediaLoaderError.missingClient } let result = try await client.getMediaFile(mediaSource: source.underlyingSource, filename: filename, mimeType: source.mimeType ?? "application/octet-stream", diff --git a/ElementX/Sources/Services/Media/Provider/MediaLoaderProtocol.swift b/ElementX/Sources/Services/Media/Provider/MediaLoaderProtocol.swift index 55666713b..b1274ead2 100644 --- a/ElementX/Sources/Services/Media/Provider/MediaLoaderProtocol.swift +++ b/ElementX/Sources/Services/Media/Provider/MediaLoaderProtocol.swift @@ -7,6 +7,10 @@ import Foundation +enum MediaLoaderError: Error { + case missingClient +} + // sourcery: AutoMockable protocol MediaLoaderProtocol { func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data diff --git a/ElementX/Sources/Services/UserSession/UserSessionStore.swift b/ElementX/Sources/Services/UserSession/UserSessionStore.swift index 55f2f3f6b..dbc5f2b10 100644 --- a/ElementX/Sources/Services/UserSession/UserSessionStore.swift +++ b/ElementX/Sources/Services/UserSession/UserSessionStore.swift @@ -94,7 +94,7 @@ class UserSessionStore: UserSessionStoreProtocol { // MARK: - Private private func buildUserSessionWithClient(_ clientProxy: ClientProxyProtocol) -> UserSessionProtocol { - let mediaProvider = MediaProvider(mediaLoader: clientProxy, + let mediaProvider = MediaProvider(mediaLoader: clientProxy.mediaLoader, imageCache: .onlyInMemory, networkMonitor: networkMonitor)