ReactiveX / RxSwift

Reactive Programming in Swift
MIT License
24.38k stars 4.17k forks source link

Strange crash after .rx_observe() usage #88

Closed iandundas closed 9 years ago

iandundas commented 9 years ago

So I have an subscription on viewModel.annotations which swaps out the <MKAnnotation>s on the map when the ViewModel's array changes:

    viewModel.annotations >- subscribeNext { [weak self] annotationsArray in
        self?.mapView.removeAnnotations(self?.mapView.annotations)
        self?.mapView.addAnnotations(annotationsArray) // marker1 
    }

This works great, the map is updated when I change that array, it's all good. Except when I add the following:

    let mapViewSignal = self.mapView.rx_observe("annotations") as Observable<[AnyObject]?>
    mapViewSignal >- subscribeNext { x in
        println("annotations did change")
    }

With this present, the code at marker1 now crashes (where before it worked great) with error:

[Swift._SwiftDeferredNSArray intersectsSet:]: unrecognized selector sent to instance 0x7f9c4a73d500

I've no idea what could cause this - why is it now calling a NSSet method on my array? And why does this only happen when I add a KVO observer (self.mapView.rx_observe("annotations")) to MKMapView?

Any help here gladly received! :)

p.s. loving RxSwift so far! The documentation is way more clear than I've found ReactiveCocoa's to be.

iandundas commented 9 years ago

p.s. the implementation of the <MKAnnotation> class is:

@objc public class PointAnnotation: NSObject, MKAnnotation{

    // MARK MKAnnotation conformance:

    // Center latitude and longitude of the annotation view.
    // The implementation of this property must be KVO compliant.
    @objc public var coordinate: CLLocationCoordinate2D{
        willSet{
            println("willSet coordinate value")
        }
        didSet{
            println("didSet coordinate value")
        }
    }

    // Title and subtitle for use by selection UI.
    public let title: String
    public let subtitle: String

    init(coordinate: CLLocationCoordinate2D, title: String, subtitle: String){
        self.coordinate = coordinate
        self.title = title
        self.subtitle = subtitle
    }
}
kzaher commented 9 years ago

Hi Ian,

I'm having troubles figuring out from the code what are you trying to do there.

I would suggest trying to print out annotationsArray before passing to that method, and figuring out the type.

KVO is an ObjC mechanism, so make sure that the object on property that you are observing is compatible with ObjC.

People usually use RxSwift.Variable on observed path, and that causes problems for them.

Hope this helps.

iandundas commented 9 years ago

Hey kzaher - thanks for replying, sorry it wasn't clear,

It is like so:

1 - When annotations:Observable<[PointAnnotation]> in the ViewModel sends a Signal (Event?), the Signal value is an array of new annotations - so the old ones are removed from mapView: MKMapView, and the new ones added.

2 - I wanted to also use KVO to observe the mapView.annotations NSArray (not sure if it's KVO compatible - I'll check, this could be the problem) so that when the mapView annotations array changes, we'd refocus the map to frame the positions of the new annotations using mapView.showAnnotations

The problem is that when I use self.mapView.rx_observe("annotations") to try to achieve step 2, I get the crash as given above. So I'm not sure how to bind to mapView.annotations.

(Step 1 works)

The code is in context here on the observe-mapview-annotations branch, should anyone want to see it.

In answer to what you suggested:

I would suggest trying to print out annotationsArray before passing to that method, and figuring out the type.

Well, it does actually update the map correctly, without the KVO stuff added. Once it's added, then I get a crash. So whatever's in the array should be okay, but it's here:

$ po annotationsArray
[<TacksSwift.PointAnnotation: 0x7fb883d268a0>]

KVO is an ObjC mechanism, so make sure that the object on property that you are observing is compatible with ObjC.

I'm observing MKMapView here - I'm assuming it supports KVO - indeed I do get an initial "annotations did change" in the console when I first set the map up with an empty array.

People usually use RxSwift.Variable on observed path, and that causes problems for them.

Not sure what you mean here exactly - you're saying people sometimes try to KVO a signal? I'm not doing that AFAIK :)

Anyway, thanks for your help so far

kzaher commented 9 years ago

Hi Ian,

you shouldn't be using KVO for code that you've written. That's meant only for legacy ObjC code.

I suggest changing your code to this.

annotations: Variable<[PointAnnotation]>

Explanation how you use variables is here

https://github.com/ReactiveX/RxSwift/blob/master/Documentation/GettingStarted.md#variables

Variables are observables that you can send values to

sendValue(annotations, [....])

Hope this helps.

iandundas commented 9 years ago

Hey kzaher - thanks for the link, I've just reread it to double-check. but I believe I've not quite explained myself properly, as I don't think this is the solution:

Please note: I'm using KVO only to observe Apple's MKMapView.annotations, never on my own code, and it is with the swizzling that I believe there's possibly a problem - here's more info:

The implementation of my annotations array (different to the map's annotations array) is here:


// Variable:
public let points:Variable<[Point]> = Variable([Point]())

// annotations is the points variable mapped to return an array of Point.annotation instead
public lazy var annotations: Observable<[PointAnnotation]> = {
    return self.points >- map { (pointArray: [Point]) in
        return pointArray.map({ (n: Point) in
            return n.annotation
        })
    }
}()

annotations is not a Variable - it is just points:Variable<[Point]>, mapped into a lazy var annotations: Observable<[PointAnnotation]>. I believe - I think - this is how to map/transform a Variable, (and it's indeed working - I get a subscribeNext when points is changed):

This Variable points is updated using points.next(newObjects). This propagates to the annotations Observable and is mapped properly - and that bit is indeed working.

The only problem is that once the KVOObservable stuff is added to Apple's MKMapView annotations property, I can no longer manually set mapView.annotations on it anymore without the app crashing - which is either a bug or I've misunderstood (thanks for your patience 👍🏻)

Here is the full stack trace, for the record:

2015-08-09 22:57:58.171 TacksSwift[76672:1803096] -[Swift._SwiftDeferredNSArray intersectsSet:]: unrecognized selector sent to instance 0x7fa318b78410
2015-08-09 22:58:02.996 TacksSwift[76672:1803096] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[Swift._SwiftDeferredNSArray intersectsSet:]: unrecognized selector sent to instance 0x7fa318b78410'
*** First throw call stack:
(
    0   CoreFoundation                      0x0000000106c76c65 __exceptionPreprocess + 165
    1   libobjc.A.dylib                     0x0000000106553bb7 objc_exception_throw + 45
    2   CoreFoundation                      0x0000000106c7e0ad -[NSObject(NSObject) doesNotRecognizeSelector:] + 205
    3   CoreFoundation                      0x0000000106bd413c ___forwarding___ + 988
    4   CoreFoundation                      0x0000000106bd3cd8 _CF_forwarding_prep_0 + 120
    5   Foundation                          0x0000000106138d76 NSKeyValueWillChangeBySetMutation + 156
    6   Foundation                          0x00000001060adb9a NSKeyValueWillChange + 386
    7   Foundation                          0x0000000106138c76 -[NSObject(NSKeyValueObserverNotification) willChangeValueForKey:withSetMutation:usingObjects:] + 225
    8   Foundation                          0x00000001061804a5 NSKVOUnionSetAndNotify + 116
    9   TacksSwift                          0x0000000105af9253 _TFFC10TacksSwift17MapViewController11viewDidLoadFS0_FT_T_U_FGSaCS_15PointAnnotation_T_ + 1347
    10  TacksSwift                          0x0000000105af92c8 _TTRXFo_oGSaC10TacksSwift15PointAnnotation__dT__XFo_iGSaS0___dT__ + 24
    11  RxSwift                             0x0000000105e3d01f _TFFF7RxSwift13subscribeNextU__FFQ_T_FGCS_10ObservableQ__PS_10Disposable_U_FGS0_Q__PS1__U_FGOS_5EventQ__T_ + 271
    12  RxSwift                             0x0000000105dbc08c _TFC7RxSwift17AnonymousObserver6onCoreU__fGS0_Q__FGOS_5EventQ__T_ + 108
    13  RxSwift                             0x0000000105e50d4e _TFC7RxSwift12ObserverBase2onU__fGS0_Q__FGOS_5EventQ__T_ + 718
    14  RxSwift                             0x0000000105e5047f _TTWU__GC7RxSwift8ObserverQ__S_12ObserverTypeS_FS1_2onUS1__U__fQPS1_FGOS_5EventQS2_7Element_T_ + 31
    15  RxSwift                             0x0000000105e53447 _TF7RxSwift11trySendNextUS_12ObserverType_U__FTGSqQ__QQ_7Element_T_ + 263
    16  RxSwift                             0x0000000105e2b295 _TFFC7RxSwift7MapSink2onU_S_12ObserverType_U__FGS0_Q_Q0__FGOS_5EventQ__T_U_FQQ0_7ElementGOS_8RxResultT__ + 149
    17  RxSwift                             0x0000000105e6d718 _TFO7RxSwift8RxResult7flatMapU__fGS0_Q__U__FFQd__GS0_Q__GS0_Q__ + 280
    18  RxSwift                             0x0000000105e288fc _TFC7RxSwift7MapSink2onU_S_12ObserverType_U__fGS0_Q_Q0__FGOS_5EventQ__T_ + 604
    19  RxSwift                             0x0000000105e28deb _TTWU_7RxSwift12ObserverType_U__GCS_7MapSinkQ_Q0__S0_S_FS0_2onUS0__U__fQPS0_FGOS_5EventQS2_7Element_T_ + 59
    20  RxSwift                             0x0000000105e50579 _TFC7RxSwift15ObserverAdapter2onUS_12ObserverType_U__fGS0_Q__FGOS_5EventQQ_7Element_T_ + 137
    21  RxSwift                             0x0000000105e5047f _TTWU__GC7RxSwift8ObserverQ__S_12ObserverTypeS_FS1_2onUS1__U__fQPS1_FGOS_5EventQS2_7Element_T_ + 31
    22  RxSwift                             0x0000000105e52333 _TF7RxSwift8dispatchUSs12SequenceType_S_12ObserverType_USs13GeneratorType___FTGOS_5EventQQ0_7Element_GSqQ___T_ + 803
    23  RxSwift                             0x0000000105dc58ee _TFC7RxSwift15BehaviorSubject2onU__fGS0_Q__FGOS_5EventQ__T_ + 654
    24  RxSwift                             0x0000000105e87ed7 _TTWU___GC7RxSwift11SubjectTypeQ_Q0__S_12ObserverTypeS_FS1_2onUS1__U__fQPS1_FGOS_5EventQS2_7Element_T_ + 39
    25  RxSwift                             0x0000000105e5313b _TF7RxSwift8sendNextUS_12ObserverType_U__FTQ_QQ_7Element_T_ + 187
    26  RxSwift                             0x0000000105ea01f2 _TFC7RxSwift8Variable4nextU__fGS0_Q__FQ_T_ + 162
    27  TacksSwift                          0x0000000105b0f4d3 _TFC10TacksSwift12MapViewModel23didChangeAdapterContentfS0_FGSaCS_5Point_T_ + 99
    28  TacksSwift                          0x0000000105b0fc77 _TTRXFo_oGSaC10TacksSwift5Point__dT__XFo_iGSaS0___iT__ + 23
    29  TacksSwift                          0x0000000105b0f3a1 _TPA__TTRXFo_oGSaC10TacksSwift5Point__dT__XFo_iGSaS0___iT__8 + 81
    30  TacksSwift                          0x0000000105adeab0 _TTRXFo_iGSaC10TacksSwift5Point__iT__XFo_oGSaS0___dT__ + 32
    31  TacksSwift                          0x0000000105adff94 _TFC10TacksSwift12ArrayAdapterW7objectsGSaCS_5Point_ + 212
    32  TacksSwift                          0x0000000105ae013f _TFC10TacksSwift12ArrayAdapters7objectsGSaCS_5Point_ + 159
    33  TacksSwift                          0x0000000105ae06f5 _TTWC10TacksSwift12ArrayAdapterS_15AdapterProtocolS_FS1_s7objectsGSaCS_5Point_ + 21
    34  TacksSwift                          0x0000000105afc8ba _TFC10TacksSwift19PointMenuController9tableViewfS0_FTCSo11UITableView23didSelectRowAtIndexPathCSo11NSIndexPath_T_ + 682
    35  TacksSwift                          0x0000000105afca6f _TToFC10TacksSwift19PointMenuController9tableViewfS0_FTCSo11UITableView23didSelectRowAtIndexPathCSo11NSIndexPath_T_ + 79
    36  UIKit                               0x0000000107418d89 -[UITableView _selectRowAtIndexPath:animated:scrollPosition:notifyDelegate:] + 1293
    37  UIKit                               0x0000000107418eca -[UITableView _userSelectRowAtPendingSelectionIndexPath:] + 219
    38  UIKit                               0x000000010734b5ec _applyBlockToCFArrayCopiedToStack + 314
    39  UIKit                               0x000000010734b466 _afterCACommitHandler + 533
    40  CoreFoundation                      0x0000000106ba9ca7 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
    41  CoreFoundation                      0x0000000106ba9c00 __CFRunLoopDoObservers + 368
    42  CoreFoundation                      0x0000000106b9fa33 __CFRunLoopRun + 1123
    43  CoreFoundation                      0x0000000106b9f366 CFRunLoopRunSpecific + 470
    44  GraphicsServices                    0x000000010d579a3e GSEventRunModal + 161
    45  UIKit                               0x00000001073278c0 UIApplicationMain + 1282
    46  TacksSwift                          0x0000000105afa44b main + 1915
    47  libdyld.dylib                       0x0000000109b31145 start + 1
)
libc++abi.dylib: terminating with uncaught exception of type NSException
iandundas commented 9 years ago

I did some more experimenting, and I'm going to open another issue, because I think this was complicated by talk of viewModels etc - the issue is more easily reproducible than that and I think indicates a bug. gimme a few minutes :)

iandundas commented 9 years ago

Ah, reopening - it's probably worth a read anyway. Would be curious if you still do think I'm using it wrong :)

kzaher commented 9 years ago

Haha :)

The method that you've described rx_observe("annotations") is just a fancy wrapper for

-(void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {

The problem with KVO is that if classes don't explicitly say they support it, then it could work, but who knows :)

I think we could make a simple test to figure out is RxSwift causing this behavior, or KVO.

Can you please try using KVO in a raw form (you can ofc use Swift version of the following code)

[mapView addObserver:self forKeyPath:@"annotations" options:options context:nil];

...

-(void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {
      NSLog(@"change: %@");
}

and let me know does that crash your code.

iandundas commented 9 years ago

cool - I'll definitely follow this up, first thing in the AM. Thanks for your time!

iandundas commented 9 years ago

Okay I tried it with regular KVO syntax and I get the same problems - so it's not rxSwift. Which is good! I should have known that mapView couldn't be KVO'd - it doesn't say it can be in the docs.

Thanks anyway :)