Skip to content
This repository has been archived by the owner on Aug 13, 2021. It is now read-only.

Commit

Permalink
Fix memory leak in ReactiveProperty.
Browse files Browse the repository at this point in the history
Summary: ReactiveProperty must hold on to its stream but its stream shouldn't hold a reference to the property.

Reviewers: O2 Material Motion, O4 Material Apple platform reviewers, #material_motion, markwei

Reviewed By: O2 Material Motion, O4 Material Apple platform reviewers, #material_motion, markwei

Subscribers: markwei

Tags: #material_motion

Differential Revision: http://codereview.cc/D2703
  • Loading branch information
jverkoey committed Feb 17, 2017
1 parent 55a6d61 commit 901e9f6
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 23 deletions.
2 changes: 1 addition & 1 deletion examples/ModalDialogExample.swift
Expand Up @@ -130,7 +130,7 @@ class ModalDialogTransitionDirector: SelfDismissingTransitionDirector {
runtime.add(spring, to: reactiveForeLayer.positionY)

if mainThreadReactive {
let rotation = reactiveForeLayer.positionY.stream
let rotation = reactiveForeLayer.positionY
.mapRange(rangeStart: spring.backwardDestination,
rangeEnd: spring.forwardDestination,
destinationStart: CGFloat(M_PI / 8),
Expand Down
2 changes: 1 addition & 1 deletion examples/SwipeExample.swift
Expand Up @@ -64,7 +64,7 @@ class TossableStackedCard: ViewInteraction {
])
runtime.add(destinationStream, to: destination)

let gestureEnabledStream = tossDirection.stream.rewrite([
let gestureEnabledStream = tossDirection.rewrite([
.none: true,
.left: false,
.right: false
Expand Down
29 changes: 12 additions & 17 deletions src/ReactiveProperty.swift
Expand Up @@ -51,21 +51,6 @@ public final class ReactiveProperty<T> {
}
}

public lazy var stream: MotionObservable<T> = {
let stream = MotionObservable<T> { observer in
self.observers.append(observer)

observer.next(self.value)

return {
if let index = self.observers.index(where: { $0 === observer }) {
self.observers.remove(at: index)
}
}
}
return stream
}()

/** Initializes a new instance with the given initial value and write function. */
public init(initialValue: T, write: @escaping ScopedWrite<T>) {
self._value = initialValue
Expand Down Expand Up @@ -108,7 +93,7 @@ public final class ReactiveProperty<T> {
private var state = MotionState.atRest
private var coreAnimationEvent: CoreAnimationChannelEvent?

private var observers: [MotionObserver<T>] = []
fileprivate var observers: [MotionObserver<T>] = []
}

public func == <T: Equatable> (left: ReactiveProperty<T>, right: T) -> Bool {
Expand All @@ -117,7 +102,17 @@ public func == <T: Equatable> (left: ReactiveProperty<T>, right: T) -> Bool {

extension ReactiveProperty: MotionObservableConvertible {
public func asStream() -> MotionObservable<T> {
return stream
return MotionObservable<T> { observer in
self.observers.append(observer)

observer.next(self.value)

return {
if let index = self.observers.index(where: { $0 === observer }) {
self.observers.remove(at: index)
}
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/interactions/Tossable.swift
Expand Up @@ -28,7 +28,7 @@ public class Destination: MotionObservableConvertible {
public let property: ReactiveProperty<CGPoint>

public func asStream() -> MotionObservable<CGPoint> {
return property.stream
return property.asStream()
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/interactions/TransitionSpring.swift
Expand Up @@ -41,8 +41,8 @@ public class TransitionSpring<T: Zeroable>: Spring<T>, TransitionInteraction {
self.forwardDestination = forwardDestination
self._initialValue = direction == .forward ? backwardDestination : forwardDestination

self.toggledDestination = direction.stream.destinations(back: backwardDestination,
fore: forwardDestination)
self.toggledDestination = direction.destinations(back: backwardDestination,
fore: forwardDestination)
super.init(threshold: threshold, system: system)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/properties/PropertyObservation.swift
Expand Up @@ -24,7 +24,7 @@ class PropertyObservation: XCTestCase {
let property = createProperty(withInitialValue: point)

var observedValue: CGPoint? = nil
let _ = property.stream.subscribe(next: { value in
let _ = property.asStream().subscribe(next: { value in
observedValue = value
}, coreAnimation: { _ in })
XCTAssertEqual(observedValue!, point)
Expand Down

0 comments on commit 901e9f6

Please sign in to comment.