1

I try to make multiple searches synchronously (I mean one after the other, waiting for the previous request to complete before running the next one) and block till all the operations are complete before going ahead.

But completion handle of the local search looks like blocked and run once the semaphore gives up. I have made many attempts without success.

My code and logs are as follows (you can copy/paste to the playground): 

import CoreLocation
import MapKit


func search(_ query: String, in span: MKCoordinateSpan, centered center: CLLocationCoordinate2D, id: Int) {

    let semaphore = DispatchSemaphore(value: 0)
    //let group = DispatchGroup(); group.enter()

    // Run the request for this rect
    print("\(#function): local search on the \(id)th portion ")

    let request = MKLocalSearch.Request()
    request.naturalLanguageQuery = query
    request.region = MKCoordinateRegion(center: center, span: span)

    if #available(iOS 13, *) {
        request.resultTypes = .pointOfInterest
    }


    let search = MKLocalSearch(request: request)

    search.start { response, error in
        print("\(id) got \(response?.mapItems.count) items")
        semaphore.signal()
    }


    let s = semaphore
    //let s = group

    // Wait for the request ot complete
    print("\(#function): waiting for the \(id)th portion to complete")
    //guard _ = s.wait(wallTimeout: .distantFuture) else {
    guard s.wait(timeout: .now() + 5) == .success else {
        print("\(#function): ***Warning: \(id)th timeout, job incomplete")
        return
    }

    print("\(#function): \(id)th completed")
}



let rect = CGRect(
    x: 48.10,
    y: 3.43,
    width: 0.09,
    height: 0.09
)


let n = 4
let latDelta = rect.width / CGFloat(n)
var latOffs = rect.minX



let queue = OperationQueue()
//queue.maxConcurrentOperationCount = 1
var ops = [BlockOperation]()

// -- Run all asyn loca search requests synchronuously
for i in 0..<n {
    // Take the next cut of the original region
    let portion = CGRect(
        x: latOffs,
        y: rect.minY,
        width: latDelta,
        height: rect.height
    )

    latOffs += latDelta

    ops.append(BlockOperation { [portion, i] in
        let center = CLLocationCoordinate2D(latitude: CLLocationDegrees(portion.midX), longitude: CLLocationDegrees(portion.midY))
        let span = MKCoordinateSpan(latitudeDelta: CLLocationDegrees(portion.width), longitudeDelta: CLLocationDegrees(portion.height))

        search("coffee", in: span, centered: center, id: i)
    })
}

queue.addOperations(ops, waitUntilFinished: true)

print("All done")

The current bogus output:

search(_:in:centered:id:): local search on the 1th portion 
search(_:in:centered:id:): local search on the 2th portion 
search(_:in:centered:id:): local search on the 3th portion 
search(_:in:centered:id:): local search on the 0th portion 
search(_:in:centered:id:): waiting for the 1th portion to complete
search(_:in:centered:id:): waiting for the 3th portion to complete
search(_:in:centered:id:): waiting for the 2th portion to complete
search(_:in:centered:id:): waiting for the 0th portion to complete
search(_:in:centered:id:): ***Warning: 0th timeout, job incomplete
search(_:in:centered:id:): ***Warning: 2th timeout, job incomplete
search(_:in:centered:id:): ***Warning: 1th timeout, job incomplete
search(_:in:centered:id:): ***Warning: 3th timeout, job incomplete
All done
0 got Optional(10) items
2 got Optional(10) items
1 got Optional(10) items
3 got Optional(10) items

[UPDATE]

The expected output should show no ***Warning and All done as the last line, as follows (the exact order of the numbering depends on the network conditions):

search(_:in:centered:id:): local search on the 1th portion 
search(_:in:centered:id:): local search on the 2th portion 
search(_:in:centered:id:): local search on the 3th portion 
search(_:in:centered:id:): local search on the 0th portion 
search(_:in:centered:id:): waiting for the 1th portion to complete
search(_:in:centered:id:): waiting for the 3th portion to complete
search(_:in:centered:id:): waiting for the 2th portion to complete
search(_:in:centered:id:): waiting for the 0th portion to complete
0 got Optional(10) items
search(_:in:centered:id:): 0th completed
2 got Optional(10) items
search(_:in:centered:id:): 2th completed
1 got Optional(10) items
search(_:in:centered:id:): 1th completed
3 got Optional(10) items
search(_:in:centered:id:): 3th completed
All done

[UPDATE 2] the outputted when uncommenting the line //queue.maxConcurrentOperationCount = 1

search(:in:centered:id:): local search on the 0th portion 2020-03-28 23:49:41 +0000 search(:in:centered:id:): waiting for the 0th portion to complete 2020-03-28 23:49:41 +0000 search(:in:centered:id:): ***Warning: 0th timeout, job incomplete 2020-03-28 23:49:46 +0000 search(:in:centered:id:): local search on the 1th portion 2020-03-28 23:49:46 +0000 search(:in:centered:id:): waiting for the 1th portion to complete 2020-03-28 23:49:46 +0000 search(:in:centered:id:): ***Warning: 1th timeout, job incomplete 2020-03-28 23:49:51 +0000 search(:in:centered:id:): local search on the 2th portion 2020-03-28 23:49:51 +0000 search(:in:centered:id:): waiting for the 2th portion to complete 2020-03-28 23:49:51 +0000 search(:in:centered:id:): ***Warning: 2th timeout, job incomplete 2020-03-28 23:49:56 +0000 search(:in:centered:id:): local search on the 3th portion 2020-03-28 23:49:56 +0000 search(:in:centered:id:): waiting for the 3th portion to complete 2020-03-28 23:49:56 +0000 search(:in:centered:id:): ***Warning: 3th timeout, job incomplete 2020-03-28 23:50:01 +0000 All done 2020-03-28 23:50:01 +0000 0 got Optional(10) items 2020-03-28 23:50:02 +0000 3 got Optional(10) items 2020-03-28 23:50:02 +0000 2 got Optional(10) items 2020-03-28 23:50:02 +0000 1 got Optional(10) items 2020-03-28 23:50:02 +0000

Note: Btw, I also added \(Date()) at the end of each print

Stéphane de Luca
  • 12,745
  • 9
  • 57
  • 95
  • You list what your expect results to be. But if you use `maxConcurrentOperationCount = 1` (which is essential to not run the searches concurrently), that is _not_ what the expected results should be. You shouldn’t see start the next search until the prior one is done. – Rob Mar 28 '20 at 23:48
  • I have updated with the output when uncommented the max = 1 for yo to see. I am looking thru your code: it's not the same situation. In yours, you make different queries. In mine, only the region changes. So I'm making changes to see if eta behave the same. – Stéphane de Luca Mar 28 '20 at 23:55
  • Yep, now that you’ve used max = 1, you’re seeing your output in order and it makes complete sense. The only thing that is curious is why they’re timing out. Have you tried this in an app rather than a playground? Have you tried bumping your timeout from 5 seconds to something much larger? – Rob Mar 29 '20 at 00:02
  • Yep, that's the original point. Why it times out. Same on a real device. Same behaviour if I set a looong timeout. – Stéphane de Luca Mar 29 '20 at 00:10
  • You could be deadlocking (which is one of the reasons we avoid every using `wait` in our code, whether semaphores, groups, or operation queues). If the thread used by the `search` completion handler is the same as the thread you’re calling from, then you have classical deadlock. Try the asynchronous `Operation` subclass pattern, which doesn’t block any threads, and therefore cannot deadlock. – Rob Mar 29 '20 at 00:26
  • The other pattern is to not use semaphores or operation queues at all and just initiate next search from the completion handler of the other (an asynchronous recursion pattern). That eliminates any possibility of deadlocks, too. – Rob Mar 29 '20 at 00:29
  • Yeah, sure, but the code is not great to maintain. I'm still investigating the codes (yours vs mine). – Stéphane de Luca Mar 29 '20 at 00:31
  • 1
    By removing setting waitUntilFinished: false, the timeout disappeared. But, of course I lose my barrier. – Stéphane de Luca Mar 29 '20 at 00:37
  • That makes sense. We never want to block threads. (That’s why I gave `performMultipleSearches` an escaping completion handler closure rather than waiting.) – Rob Mar 29 '20 at 00:41

1 Answers1

0

If you want these operations to behave in a serial manner, you have to specify that the queue can only run one at a time, e.g. 

queue.maxConcurrentOperationCount = 1

And, as you discovered, you want to avoid using waitUntilFinished option of addOperations, as that blocks the current thread until the operations are done. Instead, use completion handler pattern.

Here is the code that I used:

func performMultipleSearches(completion: @escaping () -> Void) {
    let searches = ["restaurant", "coffee", "hospital", "natural history museum"]

    let queue = OperationQueue()
    queue.maxConcurrentOperationCount = 1

    for (i, searchText) in searches.enumerated() {
        queue.addOperation {
            self.search(searchText, in: self.mapView.region, id: i)
        }
    }

    queue.addOperation {
        completion()
    }
}

func search(_ query: String, in region: MKCoordinateRegion, id: Int) {
    let semaphore = DispatchSemaphore(value: 0)

    os_log("%d starting", id)

    let request = MKLocalSearch.Request()
    request.naturalLanguageQuery = query
    request.region = region

    if #available(iOS 13, *) {
        request.resultTypes = .pointOfInterest
    }

    let search = MKLocalSearch(request: request)

    search.start { response, error in
        defer { semaphore.signal() }

        guard let mapItems = response?.mapItems else {
            os_log("  %d failed", id)
            return
        }

        os_log("  %d succeeded, found %d:", id, mapItems.count)
    }

    os_log("  %d waiting", id)
    guard semaphore.wait(timeout: .now() + 5) == .success else {
        os_log("  %d timedout", id)
        return
    }

    os_log("  %d done", id)
}

That produced:

2020-03-28 16:16:25.219565-0700 MyApp[46601:2107182] 0 starting
2020-03-28 16:16:25.220018-0700 MyApp[46601:2107182]   0 waiting
2020-03-28 16:16:25.438121-0700 MyApp[46601:2107033]   0 succeeded, found 10:
2020-03-28 16:16:25.438269-0700 MyApp[46601:2107182]   0 done
2020-03-28 16:16:25.438436-0700 MyApp[46601:2107182] 1 starting
2020-03-28 16:16:25.438566-0700 MyApp[46601:2107182]   1 waiting
2020-03-28 16:16:25.639198-0700 MyApp[46601:2107033]   1 succeeded, found 10:
2020-03-28 16:16:25.639357-0700 MyApp[46601:2107182]   1 done
2020-03-28 16:16:25.639490-0700 MyApp[46601:2107182] 2 starting
2020-03-28 16:16:25.639598-0700 MyApp[46601:2107182]   2 waiting
2020-03-28 16:16:25.822085-0700 MyApp[46601:2107033]   2 succeeded, found 10:
2020-03-28 16:16:25.822274-0700 MyApp[46601:2107182]   2 done
2020-03-28 16:16:25.822422-0700 MyApp[46601:2107162] 3 starting
2020-03-28 16:16:25.822567-0700 MyApp[46601:2107162]   3 waiting
2020-03-28 16:16:26.015566-0700 MyApp[46601:2107033]   3 succeeded, found 1:
2020-03-28 16:16:26.015696-0700 MyApp[46601:2107162]   3 done
2020-03-28 16:16:26.015840-0700 MyApp[46601:2107162] all done

For what it’s worth, I wouldn’t use semaphores and instead would use an asynchronous Operation subclass. For example, you can use the AsynchronousOperation class defined here, and then do:

class SearchOperation: AsynchronousOperation {
    let identifier: Int
    let searchText: String
    let region: MKCoordinateRegion

    init(identifier: Int, searchText: String, region: MKCoordinateRegion) {
        self.identifier = identifier
        self.searchText = searchText
        self.region = region

        super.init()
    }

    override func main() {
        os_log("%d started", identifier)

        let request = MKLocalSearch.Request()
        request.naturalLanguageQuery = searchText
        request.region = region

        if #available(iOS 13, *) {
            request.resultTypes = .pointOfInterest
        }

        let search = MKLocalSearch(request: request)

        search.start { response, error in
            defer { self.finish() }

            guard let mapItems = response?.mapItems else {
                os_log("  %d failed", self.identifier)
                return
            }

            os_log("  %d succeeded, found %d:", self.identifier, mapItems.count)
        }
    }
}

And then

let searches = ["restaurant", "coffee", "hospital", "natural history museum"]

let queue = OperationQueue()
queue.maxConcurrentOperationCount = 1

for (i, searchText) in searches.enumerated() {
    queue.addOperation(SearchOperation(identifier: i, searchText: searchText, region: mapView.region))
}

queue.addOperation {
    completion()
}
Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • I already tried this (see the comments in the code). To me it's like the trend is blocked in search.start() as the completion closure (supposed to be ran on the main thread) is never fired before the semaphore expires. Give it a try on the playground. – Stéphane de Luca Mar 28 '20 at 22:54
  • It works fine for me with `maxConcurrentOperationCount` set. – Rob Mar 28 '20 at 23:09
  • I updated the expected result so that it's clearer what to get – Stéphane de Luca Mar 28 '20 at 23:29
  • And I updated mine with my output. But the `maxConcurrentOperationCount = 1` is essential. Otherwise, it will run them concurrently. – Rob Mar 28 '20 at 23:37