MKMapView still sending messages to delegate after it's superview has been de-alloc'ed
Asked Answered
T

2

14

EDIT: changed the title. I didn't know it at the time but this is a duplicate of Why am I crashing after MKMapView is freed if I'm no longer using it?


This question is similar to Why is object not dealloc'ed when using ARC + NSZombieEnabled but different enough that I thought it worth throwing out there in case anyone understands and can explain to me what is happening. The other question may be an XCode bug so I presume this could be similar.

Scenario:

  1. RootViewController has a tableView displaying a bunch of items
  2. Selecting a cell presents a modal detailViewController containing another tableView
  3. One of the table cells in detailViewController contains an MKMapView showing the location of the item
  4. mapView.delegate = detailViewController
  5. Dismiss the modal detailViewController

Soon after this, the app crashes b/c the MKMapView sends mapView:viewForAnnotation: to the now dealloc'ed detailViewController. This crash repro'ed on a users device with an ad-hoc distribution build so the issue has nothing to do with NSZombieEnabled.

I was able to resolve the crash by adding:

_mapView.delegate = nil;

to the dealloc method of the tableViewCell containing the mapView.

QUESTION: why is it necessary to nil the delegate when the cell is dealloc'ed? It seems like the mapView should be dealloc'ed by ARC when the cell is dealloc'ed leaving this unnecessary. It is good practice to nil delegates but I didn't think it would be required in this case.

EDIT: all subviews of both detailViewController and the UITableViewCells are declared as (nonatomic, strong) properties ala:

@property (nonatomic, strong)   MKMapView *         mapView;

EDIT 2: Guess I need to get better at reading the docs. @fluchtpunkt is correct. Here's the relevant info from the MKMapView documentation:

Before releasing an MKMapView object for which you have set a delegate, remember to set that object’s delegate property to nil. One place you can do this is in the dealloc method where you dispose of the map view.

Timeserver answered 19/12, 2011 at 23:31 Comment(4)
Can you post the @strong/@weak property declarations for your detailViewController, especially the MKMapView?Awe
done. I have no @weak properties as the app targets iOS 4.3+. The mapView is in a custom UITableViewCell as a strong property.Timeserver
Sounds like the same thing as pre-ARC, as I described here: #2188598Zerla
yep, it is. nothing really to do with ARC at all, just a requirement of MKMapView that I was unaware of. Wish I had seen your question before posting. Nothing like (mis)reading the docs to humble you every now and then.Timeserver
R
19

MKMapView is not compiled with ARC and because of that the property for delegate is still declared as assign instead of weak.
From the MKMapView documentation:

@property(nonatomic, assign) id<MKMapViewDelegate> delegate

And from the Transitioning to ARC Release Notes:

You may implement a dealloc method if you need to manage resources other than releasing instance variables. You do not have to (indeed you cannot) release instance variables, but you may need to invoke [systemClassInstance setDelegate:nil] on system classes and other code that isn’t compiled using ARC.


For delegates of system classes (NS*, UI*) you have to use the "old" rule of setting delegates to nil when you deallocate the delegate object.

so add a dealloc method to your detailViewController

- (void)dealloc {
    self.mapView.delegate = nil;
}
Royden answered 19/12, 2011 at 23:51 Comment(5)
well, this is (sort of) what I did. The mapView is in a table cell so when the cell is dealloc'ed I'm now setting _mapView.delegate = nil. But still seems a little suspect b/c I have no control over when the tableCell is dealloc'ed vs when it's delegate (detailViewController) is dealloc'ed. It seems like the mapView should be dealloc'ed when it's cell is dealloc'ed already so setting it's delegate to nil then should be redundant, no?Timeserver
If you don't use the object anymore ARC has released it for you. However, it is possible that a system class has retained it too. This can always happen and since you don't have control about that you have to nil the delegates of non-ARC classes. If the mapView is part of the cell you should of course set the delegate to nil in the dealloc of the cell.Royden
I understand and it makes sense when the object being dealloc'ed can explicitly ensure that nothing is holding an unsafe_unretained reference to it. In this example that's not the case as there is no guarantee that the cell wouldn't be retained by another object keeping the mapView retained even though the mapView's delegate (detailViewController) has been dealloc'ed. Imagine an example where the cell is replaced by a different class. In any case, this seems to be working for this case so I'll likely accept your answer but I'm not convinced that this will resolve all cases. Thanks.Timeserver
as a related question, are you advocating this for all assigned delegate properties of non-ARC system classes? For example, how about a tableView itself? When the controller owning the tableView is dealloc'ed should I explicitly set tableView.dataSource = nil and tableView.delegate = nil? This is definitely something that wasn't necessary pre-ARC.Timeserver
and lastly, your answer is in fact confirmed by the mapView docs. I'll add something at the bottom of the question. Guess I should have started there. thanks again.Timeserver
E
0

While it's true that the delegates for such classes should be explicitly set to nil, doing it in dealloc is already too late. You already lost your reference to the mapview during viewDidUnload. You should do the self.mapView.delegate = nil BEFORE viewDidUnload (so probably viewWillDisappear or viewDidDisappear)

From my experience, only MKMapView and UIWebView behave this way.

Endurable answered 20/12, 2011 at 3:46 Comment(5)
that's not correct. viewDidUnload is not always called. You need to nil the delegate in both places.Timeserver
also, in this case it's impossible. the mapView is in a UITableCell not a view controller.Timeserver
My wording was wrong but I was talking about viewDidUnload as a view controller cycle stage. Even if viewDidUnload is not called the IBOutlets and other weak UIView references will zero out after the view hierarchy is unloaded. (Unless you're keeping a strong reference to your views, which you probably shouldn't be doing in the first place)Endurable
I'll clarify. I appreciate your addition to the answer. WRT viewDidUnload, you do need set mapView.delegate = nil there as well. The incorrect part is to say dealloc is too late. You can't rely solely on viewDidUnload as it is not always called.Timeserver
As long as your self.mapView is still valid by then. Also, I missed your comment to fluchtpunkt that said The mapView is in a custom UITableViewCell as a strong property. so I mentioned about the view*** methods.Endurable

© 2022 - 2024 — McMap. All rights reserved.