Address the real lifetime issue of the SDK's Client by making Context.mediaProvider weak. (#4466)

This reverts commit 927aea4bfe.
This commit is contained in:
Doug
2025-09-05 18:18:14 +01:00
committed by GitHub
parent f30f3b1a63
commit c794144ef1
58 changed files with 380 additions and 161 deletions
@@ -17,7 +17,7 @@ class ClientProxy: ClientProxyProtocol {
private let networkMonitor: NetworkMonitorProtocol
private let appSettings: AppSettings
let mediaLoader: MediaLoaderProtocol
private let mediaLoader: MediaLoaderProtocol
private let clientQueue: DispatchQueue
private var roomListService: RoomListService
@@ -1113,6 +1113,20 @@ 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
@@ -73,7 +73,7 @@ enum TimelineMediaVisibility: Decodable {
}
// sourcery: AutoMockable
protocol ClientProxyProtocol: AnyObject {
protocol ClientProxyProtocol: AnyObject, MediaLoaderProtocol {
var actionsPublisher: AnyPublisher<ClientProxyAction, Never> { get }
var loadingStatePublisher: CurrentValuePublisher<ClientProxyLoadingState, Never> { get }
@@ -103,8 +103,6 @@ protocol ClientProxyProtocol: AnyObject {
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
@@ -16,17 +16,7 @@ private final class MediaRequest {
}
actor MediaLoader: MediaLoaderProtocol {
// 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 let client: ClientProtocol
private var ongoingRequests = [MediaSourceProxy: MediaRequest]()
init(client: ClientProtocol) {
@@ -34,21 +24,18 @@ actor MediaLoader: MediaLoaderProtocol {
}
func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data {
try await enqueueLoadMediaRequest(forSource: source) { [weak client] in
guard let client else { throw MediaLoaderError.missingClient }
return try await client.getMediaContent(mediaSource: source.underlyingSource)
try await enqueueLoadMediaRequest(forSource: source) {
try await self.client.getMediaContent(mediaSource: source.underlyingSource)
}
}
func loadMediaThumbnailForSource(_ source: MediaSourceProxy, width: UInt, height: UInt) async throws -> Data {
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))
try await enqueueLoadMediaRequest(forSource: source) {
try await self.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",
@@ -7,10 +7,6 @@
import Foundation
enum MediaLoaderError: Error {
case missingClient
}
// sourcery: AutoMockable
protocol MediaLoaderProtocol {
func loadMediaContentForSource(_ source: MediaSourceProxy) async throws -> Data
@@ -9,7 +9,7 @@ import Combine
import Kingfisher
import UIKit
struct MediaProvider: MediaProviderProtocol {
class MediaProvider: MediaProviderProtocol {
private let mediaLoader: MediaLoaderProtocol
private let imageCache: Kingfisher.ImageCache
private let networkMonitor: NetworkMonitorProtocol?
@@ -17,7 +17,7 @@ enum MediaProviderError: Error {
}
// sourcery: AutoMockable
protocol MediaProviderProtocol {
protocol MediaProviderProtocol: AnyObject {
func imageFromSource(_ source: MediaSourceProxy?, size: CGSize?) -> UIImage?
func loadImageFromSource(_ source: MediaSourceProxy, size: CGSize?) async -> Result<UIImage, MediaProviderError>
func loadImageDataFromSource(_ source: MediaSourceProxy) async -> Result<Data, MediaProviderError>
@@ -94,7 +94,7 @@ class UserSessionStore: UserSessionStoreProtocol {
// MARK: - Private
private func buildUserSessionWithClient(_ clientProxy: ClientProxyProtocol) -> UserSessionProtocol {
let mediaProvider = MediaProvider(mediaLoader: clientProxy.mediaLoader,
let mediaProvider = MediaProvider(mediaLoader: clientProxy,
imageCache: .onlyInMemory,
networkMonitor: networkMonitor)