From de80705deb1221a93d423f072e1cdfab233731b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Sun, 12 Jul 2026 22:20:09 +0200 Subject: [PATCH] fix(offline): serialize tileRegionPacks access on main and keep OfflineManager alive until its tasks complete The loadStylePack/loadTileRegion progress callbacks are delivered on Mapbox SDK background threads ("com.mapbox.common.MB TileStore RW") and mutated the tileRegionPacks dictionary concurrently with the main thread, corrupting it and aborting with "object deallocated with non-zero retain count" or malloc "pointer being freed was not allocated". The RCT methods (getPackStatus, resumePackDownload, pausePackDownload, deletePack) run on the module's background method queue and touched the same state, as did convertRegionsToJSON's TileStore callbacks. All of these now hop to the main queue, matching the completion handlers. Also keep the function-local OfflineManager alive until both of the async tasks it started complete: it was deallocated ~0ms after startLoading returned, before the style pack load finished. Fixes #4252 --- ios/RNMBX/Offline/RNMBXOfflineModule.swift | 141 ++++++++++++--------- 1 file changed, 83 insertions(+), 58 deletions(-) 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() + } } }