diff --git a/ios/RNMBX/Offline/RNMBXOfflineModule.swift b/ios/RNMBX/Offline/RNMBXOfflineModule.swift index 105309565c..1253ba1430 100644 --- a/ios/RNMBX/Offline/RNMBXOfflineModule.swift +++ b/ios/RNMBX/Offline/RNMBXOfflineModule.swift @@ -195,55 +195,63 @@ class RNMBXOfflineModule: RCTEventEmitter { func getPackStatus(_ name: String, resolver: @escaping RCTPromiseResolveBlock, rejecter: @escaping RCTPromiseRejectBlock) { - guard tileRegionPacks[name] != nil else { - rejecter("RNMBXOfflineModule.getPackStatus", "pack \(name) not found", nil) - return - } - - tileStore.tileRegion(forId: name) { result in - switch result { - case .success(let region): - self.tileStore.tileRegionMetadata(forId: name) { result in - switch result { - case .success(let metadata): - let pack = TileRegionPack( - name: name, - progress: self.toProgress(region: region), - metadata: logged("RNMBXOfflineModule.getPackStatus") { metadata as? [String:Any] } ?? [:] - ) - self.tileRegionPacks[name] = pack - resolver(self._makeRegionStatusPayload(pack: pack)) - case .failure(let error): - Logger.log(level:.error, message: "Unable to fetch metadata for \(name)") - rejecter("RNMBXOfflineModule.getPackStatus", error.localizedDescription, error) + DispatchQueue.main.async { + guard self.tileRegionPacks[name] != nil else { + rejecter("RNMBXOfflineModule.getPackStatus", "pack \(name) not found", nil) + return + } + + self.tileStore.tileRegion(forId: name) { result in + switch result { + case .success(let region): + self.tileStore.tileRegionMetadata(forId: name) { result in + DispatchQueue.main.async { + switch result { + case .success(let metadata): + let pack = TileRegionPack( + name: name, + progress: self.toProgress(region: region), + metadata: logged("RNMBXOfflineModule.getPackStatus") { metadata as? [String:Any] } ?? [:] + ) + self.tileRegionPacks[name] = pack + resolver(self._makeRegionStatusPayload(pack: pack)) + case .failure(let error): + Logger.log(level:.error, message: "Unable to fetch metadata for \(name)") + rejecter("RNMBXOfflineModule.getPackStatus", error.localizedDescription, error) + } + } } + case .failure(let error): + Logger.log(level:.error, message: "Unable to fetch region for \(name)") + rejecter("RNMBXOfflineModule.getPackStatus", error.localizedDescription, error) } - case .failure(let error): - Logger.log(level:.error, message: "Unable to fetch region for \(name)") - rejecter("RNMBXOfflineModule.getPackStatus", error.localizedDescription, error) } } } @objc - func resumePackDownload(_ name: String, resolver: RCTPromiseResolveBlock, rejecter: RCTPromiseRejectBlock) + func resumePackDownload(_ name: String, resolver: @escaping RCTPromiseResolveBlock, rejecter: @escaping RCTPromiseRejectBlock) { - if let pack = tileRegionPacks[name] { - startLoading(pack: pack) - resolver(nil) - } else { - rejecter("resumePackDownload", "Unknown offline pack: \(name)", nil) + DispatchQueue.main.async { + if let pack = self.tileRegionPacks[name] { + self.startLoading(pack: pack) + resolver(nil) + } else { + rejecter("resumePackDownload", "Unknown offline pack: \(name)", nil) + } } } @objc - func pausePackDownload(_ name: String, resolver: RCTPromiseResolveBlock, rejecter: RCTPromiseRejectBlock) + func pausePackDownload(_ name: String, resolver: @escaping RCTPromiseResolveBlock, rejecter: @escaping RCTPromiseRejectBlock) { - if let pack = tileRegionPacks[name] { - pack.cancelables.forEach { $0.cancel() } - resolver(nil) - } else { - rejecter("pausePackDownload", "Unknown offline region: \(name)", nil) + DispatchQueue.main.async { + if let pack = self.tileRegionPacks[name] { + pack.cancelables.forEach { $0.cancel() } + resolver(nil) + } else { + rejecter("pausePackDownload", "Unknown offline region: \(name)", nil) + } } } @@ -255,20 +263,22 @@ class RNMBXOfflineModule: RCTEventEmitter { @objc func deletePack(_ name: String, - resolver: RCTPromiseResolveBlock, - rejecter: RCTPromiseRejectBlock) + resolver: @escaping RCTPromiseResolveBlock, + rejecter: @escaping RCTPromiseRejectBlock) { - guard let pack = tileRegionPacks[name] else { - return resolver(nil) - } - - guard pack.state != .invalid else { - return rejecter("deletePack", "Pack: \(name) has already been deleted", nil) + DispatchQueue.main.async { + guard let pack = self.tileRegionPacks[name] else { + return resolver(nil) + } + + guard pack.state != .invalid else { + return rejecter("deletePack", "Pack: \(name) has already been deleted", nil) + } + + self.tileStore.removeTileRegion(forId: name) + self.tileRegionPacks[name]!.state = .invalid + resolver(nil) } - - tileStore.removeTileRegion(forId: name) - tileRegionPacks[name]!.state = .invalid - resolver(nil) } @objc @@ -358,7 +368,11 @@ class RNMBXOfflineModule: RCTEventEmitter { let threadedOfflineManager = OfflineManager() taskGroup.enter() let stylePackTask = threadedOfflineManager.loadStylePack(for: styleURI, loadOptions: stylePackLoadOptions) { progress in - self.tileRegionPacks[id]!.state = .active + // progress callbacks arrive on SDK background threads; tileRegionPacks + // is main-thread state (see #4252) + DispatchQueue.main.async { + self.tileRegionPacks[id]!.state = .active + } } completion: { result in DispatchQueue.main.async { defer { @@ -396,10 +410,14 @@ class RNMBXOfflineModule: RCTEventEmitter { taskGroup.enter() let task = self.tileStore.loadTileRegion(forId: id, loadOptions: loadOptions!, progress: { progress in - lastProgress = progress - self.tileRegionPacks[id]!.progress = progress - self.tileRegionPacks[id]!.state = .active - self.offlinePackProgressDidChange(progress: progress, metadata: metadata, state: .active) + // delivered on the "MB TileStore RW" thread; mutating tileRegionPacks + // here races the main thread and corrupts the dictionary (#4252) + DispatchQueue.main.async { + lastProgress = progress + self.tileRegionPacks[id]!.progress = progress + self.tileRegionPacks[id]!.state = .active + self.offlinePackProgressDidChange(progress: progress, metadata: metadata, state: .active) + } }) { result in DispatchQueue.main.async { defer { @@ -427,6 +445,9 @@ class RNMBXOfflineModule: RCTEventEmitter { taskGroup.notify(queue: .main) { self.tileRegionPacks[id]!.cancelables = [] self.tileRegionPacks[id]!.state = downloadError ? .inactive : .complete + // the style pack load and the tileset descriptor are tied to this + // manager; it must outlive the in-flight work (#4252) + withExtendedLifetime(threadedOfflineManager) {} } self.tileRegionPacks[id]!.cancelables = [task, stylePackTask] @@ -442,13 +463,17 @@ class RNMBXOfflineModule: RCTEventEmitter { taskGroup.enter() taskGroup.enter() tileStore.tileRegionGeometry(forId: region.id) { (result) in - geomteryResults[region.id] = (result, region) - taskGroup.leave() + DispatchQueue.main.async { + geomteryResults[region.id] = (result, region) + taskGroup.leave() + } } - + tileStore.tileRegionMetadata(forId: region.id) { (result) in - metadataResults[region.id] = result - taskGroup.leave() + DispatchQueue.main.async { + metadataResults[region.id] = result + taskGroup.leave() + } } }