Skip to content

Commit

Permalink
iOS: fix memory leak issues (#486)
Browse files Browse the repository at this point in the history
fix memory leak issues
  • Loading branch information
nancywu1 authored Aug 8, 2024
1 parent 0b074ea commit ebdcb61
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 14 deletions.
28 changes: 19 additions & 9 deletions ios/core/Sources/Player/HeadlessPlayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,7 @@ public protocol HeadlessPlayer {
public extension HeadlessPlayer {
/// The current state of Player
var state: BaseFlowState? {
guard
let jsState = jsPlayerReference?.invokeMethod("getState", withArguments: [])
else { return nil }
return BaseFlowState.createInstance(value: jsState)
return jsPlayerReference?.getState()
}

/**
Expand Down Expand Up @@ -221,6 +218,10 @@ public extension HeadlessPlayer {
- completion: A completion handler for when the flow has completed
*/
func start(flow: String, completion: @escaping (Result<CompletedState, PlayerError>) -> Void) {
// declare these variables outside and reference them inside the errorHandler to prevent retain cycle
let playerRef = jsPlayerReference
let loggerRef = logger

let promiseHandler: @convention(block) (JSValue?) -> Void = { completedState in
guard
let result = CompletedState.createInstance(from: completedState)
Expand All @@ -231,17 +232,17 @@ public extension HeadlessPlayer {
}
let errorHandler: @convention(block) (JSValue?) -> Void = { _ in
guard
let result = self.state as? ErrorState
else {
let result = playerRef?.getState() as? ErrorState else {
return completion(.failure(PlayerError.jsConversionFailure))
}
logger.e(result.error)

loggerRef.e(result.error)
completion(.failure(PlayerError.promiseRejected(error: result)))
}

// Ensure these get created because otherwise we will never know when the flow ends/errors
guard
let context = jsPlayerReference?.context,
let context = playerRef?.context,
let flowObject = context.evaluateScript("(\(flow))"),
!flowObject.isUndefined,
let callback = JSValue(object: promiseHandler, in: context),
Expand All @@ -251,7 +252,7 @@ public extension HeadlessPlayer {
}

// Should not be possible due to fatalError in constructor, but just for handling optionals safely
guard let player = jsPlayerReference else { return completion(.failure(PlayerError.playerNotInstantiated)) }
guard let player = playerRef else { return completion(.failure(PlayerError.playerNotInstantiated)) }
player
.invokeMethod("start", withArguments: [flowObject])
.invokeMethod("then", withArguments: [callback])
Expand Down Expand Up @@ -347,3 +348,12 @@ internal extension JSContext {
return value
}
}

private extension JSValue {
// put getState in extension so it can be accessed by the computed property in HeadlessPlayer.state and also inside the ErrorHandler by the playerReference
func getState() -> BaseFlowState? {
guard let jsState = self.invokeMethod("getState", withArguments: [])
else { return nil }
return BaseFlowState.createInstance(value: jsState)
}
}
3 changes: 3 additions & 0 deletions ios/swiftui/Sources/ManagedPlayer/ManagedPlayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ internal struct ManagedPlayer14<Loading: View, Fallback: View>: View {

public var body: some View {
bodyContent(viewModel.stateTransition.call() ?? .identity)
.onDisappear {
context.clearExceptionHandler()
}
}

private func bodyContent(_ transitionInfo: PlayerViewTransition) -> some View {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ public class ManagedPlayerViewModel: ObservableObject, NativePlugin {
}

public func apply<P>(player: P) where P: HeadlessPlayer {
player.hooks?.state.tap({ (state) in
player.hooks?.state.tap({ [weak self] (state) in
guard let inProgress = state as? InProgressState else {
self.currentState = nil
self?.currentState = nil
return
}
self.currentState = inProgress
self?.currentState = inProgress
})
}

Expand Down
10 changes: 8 additions & 2 deletions ios/swiftui/Sources/SwiftUIPlayer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ public struct SwiftUIPlayer: View, HeadlessPlayer {
self.onViewController(controller)
}

hooks.state.tap { newState in
self.state = newState
hooks.state.tap { [weak self] newState in
self?.state = newState
}

guard !flow.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
Expand All @@ -109,6 +109,12 @@ public struct SwiftUIPlayer: View, HeadlessPlayer {
registry.resetView()
}

/// Clear the exceptionHandler of the context to remove reference to the logger
/// should be called when ManagedPlayer gets tore down
public func clearExceptionHandler() {
player?.context.exceptionHandler = nil
}

/// Returns `player` but asserts that it is not nil. Used from methods that should not be called
/// when we are unloaded.
private var expectedPlayer: JSValue? {
Expand Down

0 comments on commit ebdcb61

Please sign in to comment.