DEPRECATED-mapbox-ios-sdk icon indicating copy to clipboard operation
DEPRECATED-mapbox-ios-sdk copied to clipboard

Synchronous requests potentially block main thread

Open RubioV opened this issue 12 years ago • 6 comments

Expanding on https://github.com/mapbox/mapbox-ios-sdk/issues/259

In RMConfiguration.h, there are a few functions which make calls to NSURLConnection sendSynchronousRequest:. These functions are used in various init routines for tile sources (and potentially other places as well). Since these functions are often invoked on the main thread, they will potentially block the UI if they take long to complete, such as over a poor network connection. Apple specifically states that sendSynchronousRequest should not be used on the main thread:

Important Because this call can potentially take several minutes to fail (particularly when using a cellular network in iOS), you should never call this function from the main thread of a GUI application.

I do not know an easy fix for this. Because the init functions for tile sources need the results of these network requests, they need to wait for the result to come back. So potentially all tile sources should be initialised on a separate thread. Perhaps there are some other ideas for fixing this?

RubioV avatar Aug 08 '13 16:08 RubioV

I do not know an easy fix for this.

Exactly :-/ However, I'm glad you're bringing it up and it's good to have a formal discussion about it.

One thing that occurs to me is to use the default bundled TileJSON or an OSM source in the first init pass, then asynchronously (at least attempt an) update to the ultimate source.

incanus avatar Aug 08 '13 22:08 incanus

Hmm, I have an idea. I've looked at RMBingSource which handles it a bit differently. The JSON data is only retrieved when the first tile URL is being requested and then saved for later use. Since the retrieval of tiles happens off the main thread this is a better solution.

Also, I've done some further digging and the only place where this is really blocking is the various init methods of RMMapBoxSource, and perhaps it's also blocking in RMInteractiveSource.m, but I'm not sure, I don't quite understand that one yet.

Perhaps in RMMapBoxSource we could defer the JSON initialization until the first tile URL request as well?

RubioV avatar Aug 08 '13 22:08 RubioV

Well, I wrote RMBingSource ;-)

Thing is, you need the TileJSON to know the valid zoom range, bounding box, and some other details, even before tile requests are made.

I wrote RMInteractiveSource as well, and this shouldn't be relevant. It does per-tile downloads of essentially compressed JSON data to do interactivity lookups. So it works like raster tiles, but it does it via category methods, so the init is already taken care of in the relevant tile source.

incanus avatar Aug 08 '13 22:08 incanus

Ok, that makes sense. The problem I see with using default values from a bundled TileJSON and then updating them asynchronously, is that you could get unexpected results. At the very least, RMMapView and RMTileCache would need to be updated so that they can be notified when the bounding box and zoom parameters get updated asynchronously. But developers could run into unexpected issues when they init and RMMapBoxSource and immediately use its zoom values etc (I know I do that for example).

How about this. Rather than a simple init method, implement something like:

typedef void (^RMMapBoxSourceCallbackHandler)(RMMapBoxSource *mapBoxSource, NSError *error);

+ (void)requestMapBoxSourceWithMapID:(NSString *)mapID withCallBack:(RMMapBoxSourceCallbackHandler)handler;

This method will then run on a separate thread, and execute the callback block on the main thread in order to notify the caller. The user is then encouraged in the documentation to save the TileJSON after the first successful init and use that for a simple init in future instances. What do you think?

RubioV avatar Aug 09 '13 11:08 RubioV

I like the concept, but I need to do some testing to see how it works in practice. I want to rethink the offline behavior a bit more.

incanus avatar Oct 18 '13 22:10 incanus

A lot of work is going into this thinking over in http://github.com/mapbox/mbxmapkit, which wrestles with the same problems relating to TileJSON, marker GeoJSON, and marker imagery being unavailable synchronously due to being network-hosted, but needing to set various properties under the current model of operation. Let's let the dust settle there a bit before moving forward in this project.

incanus avatar Mar 18 '14 00:03 incanus