Hang due to malformed shield image URL

[1ec5]

If a VisualInstructionComponent has an unreachable URL as its imageURL, the application will hang as InstructionPresenter and ImageRepository effectively encounter infinite recursion with the stack below, which includes the stack for a queued dispatch operation:

Thread 1 Queue : com.apple.main-thread (serial)
#0	0x000000010f9f10fa in ImageRepository.imageWithURL(_:cacheKey:completion:) at /path/to/mapbox-navigation-ios/MapboxNavigation/ImageRepository.swift:43
#1	0x000000010f9e9b83 in InstructionPresenter.shieldImageForComponent(_:in:height:completion:) at /path/to/mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift:166
#2	0x000000010f9e98c8 in InstructionPresenter.attributedString(forShieldComponent:repository:dataSource:onImageDownload:) at /path/to/mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift:150
#3	0x000000010f9e7b51 in InstructionPresenter.attributedPairs(for:dataSource:imageRepository:onImageDownload:) at /path/to/mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift:110
#4	0x000000010f9e4516 in InstructionPresenter.fittedAttributedComponents() at /path/to/mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift:40
#5	0x000000010f9e4008 in InstructionPresenter.attributedText() at /path/to/mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift:34
#6	0x000000010f9ebbb0 in closure #1 in InstructionPresenter.completeShieldDownload(_:) at /path/to/mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift:242
#7	0x000000010f9eff9c in partial apply for closure #1 in InstructionPresenter.completeShieldDownload(_:) ()
#8	0x000000010f898e1d in thunk for @escaping @callee_guaranteed () -> () ()
#9	0x00000001165be595 in _dispatch_call_block_and_release ()
#10	0x00000001165bf602 in _dispatch_client_callout ()
#11	0x00000001165cc99a in _dispatch_main_queue_callback_4CF ()
#12	0x00000001121743e9 in __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ ()
#13	0x000000011216ea76 in __CFRunLoopRun ()
#14	0x000000011216de11 in CFRunLoopRunSpecific ()
#15	0x0000000115c651dd in GSEventRunModal ()
#16	0x000000011e18c81d in UIApplicationMain ()
#17	0x000000010f4eb894 in main at /path/to/mapbox-navigation-ios/Example/AppDelegate.swift:6
#18	0x0000000116635575 in start ()
#19	0x0000000116635575 in start ()
Enqueued from com.mapbox.MapboxNavigation.ImageDownloadCompletionBarrierQueue (Thread 0) Queue : com.mapbox.MapboxNavigation.ImageDownloadCompletionBarrierQueue (serial)
#0	0x00000001165c3bdb in dispatch_async ()
#1	0x0000000113bf964b in OS_dispatch_queue.async(group:qos:flags:execute:) ()
#2	0x000000010f9ebb0c in InstructionPresenter.completeShieldDownload(_:) at /path/to/mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift:241
#3	0x000000010f9f025c in partial apply ()
#4	0x000000010f9f16c8 in closure #1 in ImageRepository.imageWithURL(_:cacheKey:completion:) at /path/to/mapbox-navigation-ios/MapboxNavigation/ImageRepository.swift:45
#5	0x000000010f9f1766 in partial apply for closure #1 in ImageRepository.imageWithURL(_:cacheKey:completion:) ()
#6	0x000000010f8fe633 in thunk for @escaping @callee_guaranteed (@guaranteed UIImage?, @guaranteed Data?, @guaranteed Error?) -> () ()
#7	0x000000010f901641 in partial apply for thunk for @escaping @callee_guaranteed (@guaranteed UIImage?, @guaranteed Data?, @guaranteed Error?) -> () ()
#8	0x000000010f900894 in thunk for @escaping @callee_guaranteed (@in_guaranteed (UIImage?, Data?, Error?)) -> (@out ()) ()
#9	0x000000010f90157a in partial apply for thunk for @escaping @callee_guaranteed (@in_guaranteed (UIImage?, Data?, Error?)) -> (@out ()) ()
#10	0x000000010f90070b in closure #1 in closure #1 in ImageDownloadOperation.fireAllCompletions(_:data:error:) at /path/to/mapbox-navigation-ios/MapboxNavigation/ImageDownload.swift:157
#11	0x000000010f901476 in partial apply for closure #1 in closure #1 in ImageDownloadOperation.fireAllCompletions(_:data:error:) ()
#12	0x000000010f9007a2 in thunk for @callee_guaranteed (@guaranteed @escaping @callee_guaranteed (@guaranteed UIImage?, @guaranteed Data?, @guaranteed Error?) -> ()) -> (@error @owned Error) ()
#13	0x000000010f9014bb in partial apply for thunk for @callee_guaranteed (@guaranteed @escaping @callee_guaranteed (@guaranteed UIImage?, @guaranteed Data?, @guaranteed Error?) -> ()) -> (@error @owned Error) ()
#14	0x0000000113574560 in Sequence.forEach(_:) ()
#15	0x000000010f900643 in closure #1 in ImageDownloadOperation.fireAllCompletions(_:data:error:) at /path/to/mapbox-navigation-ios/MapboxNavigation/ImageDownload.swift:157
#16	0x000000010f901331 in partial apply for closure #1 in ImageDownloadOperation.fireAllCompletions(_:data:error:) ()
#17	0x000000010f9008bc in thunk for @callee_guaranteed () -> () ()
#18	0x000000010f901371 in partial apply for thunk for @callee_guaranteed () -> () ()
#19	0x000000010f9008e1 in thunk for @escaping @callee_guaranteed () -> () ()
#20	0x00000001165bf602 in _dispatch_client_callout ()
#21	0x00000001165cdd73 in _dispatch_sync_invoke_and_complete ()
#22	0x000000010f9001d3 in ImageDownloadOperation.fireAllCompletions(_:data:error:) at /path/to/mapbox-navigation-ios/MapboxNavigation/ImageDownload.swift:156
#23	0x000000010f8ffdb4 in ImageDownloadOperation.urlSession(_:task:didCompleteWithError:) at /path/to/mapbox-navigation-ios/MapboxNavigation/ImageDownload.swift:138
#24	0x000000010f8fff93 in @objc ImageDownloadOperation.urlSession(_:task:didCompleteWithError:) ()
#25	0x000000010f9f6a6b in partial apply ()
#26	0x000000010f9f67ea in thunk for @escaping @callee_guaranteed (@unowned NSURLSession, @unowned NSURLSessionTask, @unowned NSError?) -> () ()
#27	0x000000010f9f6ad1 in partial apply for thunk for @escaping @callee_guaranteed (@unowned NSURLSession, @unowned NSURLSessionTask, @unowned NSError?) -> () ()
#28	0x000000010f9f65c7 in ImageDownloader.urlSession(_:task:didCompleteWithError:) at /path/to/mapbox-navigation-ios/MapboxNavigation/ImageDownloader.swift:113
#29	0x000000010f9f6b73 in @objc ImageDownloader.urlSession(_:task:didCompleteWithError:) ()
#30	0x0000000116ff2342 in __51-[NSURLSession delegate_task:didCompleteWithError:]_block_invoke.241 ()
#31	0x000000011121af9e in __NSBLOCKOPERATION_IS_CALLING_OUT_TO_A_BLOCK__ ()
#32	0x000000011121aea5 in -[NSBlockOperation main] ()
#33	0x0000000111217c14 in -[__NSOperationInternal _start:] ()
#34	0x000000011121dc4b in __NSOQSchedule_f ()
#35	0x00000001165be595 in _dispatch_call_block_and_release ()
#36	0x00000001165bf602 in _dispatch_client_callout ()
#37	0x00000001165c254d in _dispatch_continuation_pop ()
#38	0x00000001165c1927 in _dispatch_async_redirect_invoke ()
#39	0x00000001165d000a in _dispatch_root_queue_drain ()
#40	0x00000001165d09af in _dispatch_worker_thread2 ()
#41	0x00000001169ae70e in _pthread_wqthread ()
#42	0x00000001169ae435 in start_wqthread ()

Generating an attributed string for the instruction kicks off a request for the shield image, yet the completion handler for that request in turn tries to generate an attributed string:

mapbox-navigation-ios/MapboxNavigation/ImageRepository.swift

Line 43 in 14b3c37

let _ = imageDownloader.downloadImage(with: imageURL, completion: { [weak self] (image, data, error) in

mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift

Line 242 in 14b3c37

self.onShieldDownload?(self.attributedText()) //FIXME: Can we work with the image directly?

In the case where an image can be downloaded from the given URL, the only thing preventing infinite recursion is the image cache. But when the URL is unreachable, the cache can’t break out of the recursion:

mapbox-navigation-ios/MapboxNavigation/ImageRepository.swift

Lines 38 to 41 in 14b3c37

	if let cachedImage = cachedImageForKey(cacheKey) {
	completion(cachedImage)
	return
	}

It’s possible for VisualInstructionComponent.imageURL to be a bogus URL like @2x.png when performing client-side routing (#1808). The client-side routing engine in MapboxNavigationNative currently returns an empty string as the imageBaseURL. That alone would be a malformed URL string, so URL(string:) would bail, heading off this issue:

mapbox-navigation-ios/MapboxNavigation/InstructionPresenter.swift

Line 162 in 14b3c37

guard let imageURL = component.imageURL, let shieldKey = component.cacheKey else {

However, MapboxDirections.swift appends an image resolution and file extension onto the URL string. URL interprets @2x.png as a valid relative file path, but of course HTTPURLConnection can’t connect to such a URL. mapbox/mapbox-directions-swift#322 accounts for the special case of an empty URL string, but we should also fix the infinite recursion in this SDK, because a network error could just as easily hang the application.

/cc @mapbox/navigation-ios @kevinkreiser

[akitchen]

It seems like we need an escape hatch from the recursion here, or to break this up somehow.