maps icon indicating copy to clipboard operation
maps copied to clipboard
verified publisher icon rnmapbox

fix: make mapboxMap property optional in map view accessor

Open krystofcelba opened this issue 2 years ago • 5 comments

Description

Hi, I have recently encoutered this crash caused by some race condition when the JS code wants to access the zoom but the mapboxMap is not initialized yet. So I have made this fix. I simply changed the mapboxMap getter of RNMBXMapView to optional.

Checklist

  • [X] I've read CONTRIBUTING.md
  • [X] I updated the doc/other generated code with running yarn generate in the root folder
  • [X] I have tested the new feature on /example app.
    • [X] In V11 mode/ios
    • [X] In New Architecture mode/ios
    • [ ] In V11 mode/android
    • [ ] In New Architecture mode/android
  • [ ] I added/updated a sample - if a new feature was implemented (/example)

krystofcelba avatar Feb 14 '24 15:02 krystofcelba

@krystofcelba thanks much for looking into. My main concern is that with those fixes, we'll just skip some operations. From one side this is better than crashing, from other side it's worse as debugging a camera change that doesn't happens if much more harder to debug than a crash. We should at least ad error logs at places we don't expect MapboxMap to be nil.

Can you share a stack trace or anything regarding the original issue?

mfazekas avatar Feb 14 '24 18:02 mfazekas

I've the exact same issue on iOS with latest update.

Gp2mv3 avatar May 03 '24 16:05 Gp2mv3

@Gp2mv3 can you reproduce the issue? Can you share a stack trace?

mfazekas avatar May 04 '24 05:05 mfazekas

Yes I did, here is the stacktrace (before applying this PR):

* thread #1, queue = 'com.apple.main-thread', stop reason = Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value
  frame #0: 0x0000000185084a2c libswiftCore.dylib`_swift_runtime_on_report
  frame #1: 0x000000018512da94 libswiftCore.dylib`_swift_stdlib_reportFatalErrorInFile + 208
  frame #2: 0x0000000184d03fa4 libswiftCore.dylib`closure #1 (Swift.UnsafeBufferPointer<Swift.UInt8>) -> () in closure #1 (Swift.UnsafeBufferPointer<Swift.UInt8>) -> () in closure #1 (Swift.UnsafeBufferPointer<Swift.UInt8>) -> () in Swift._assertionFailure(_: Swift.StaticString, _: Swift.StaticString, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 292
  frame #3: 0x0000000184d03e3c libswiftCore.dylib`closure #1 (Swift.UnsafeBufferPointer<Swift.UInt8>) -> () in closure #1 (Swift.UnsafeBufferPointer<Swift.UInt8>) -> () in Swift._assertionFailure(_: Swift.StaticString, _: Swift.StaticString, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 332
  frame #4: 0x0000000184d037c8 libswiftCore.dylib`Swift._assertionFailure(_: Swift.StaticString, _: Swift.StaticString, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 184
 * frame #5: 0x0000000103f95d30 Seety`RNMBXMapView.mapboxMap.getter(self=0x0000000127463be0) at RNMBXMapView.swift:210:11
  frame #6: 0x0000000103fc7cb8 Seety`static RNMBXMapViewManager.withMapboxMap(view=0x0000000127463be0, name="getZoom", rejecter=0x0000000103fcdcec Seety`reabstraction thunk helper from @escaping @callee_unowned @convention(block) (@unowned Swift.Optional<__C.NSString>, @unowned Swift.Optional<__C.NSString>, @unowned Swift.Optional<__C.NSError>) -> () to @escaping @callee_guaranteed (@guaranteed Swift.Optional<Swift.String>, @guaranteed Swift.Optional<Swift.String>, @guaranteed Swift.Optional<Swift.Error>) -> ()partial apply forwarder with unmangled suffix ".57" at <compiler-generated>, fn=0x0000000103fc99b8 Seety`partial apply forwarder for closure #1 (MapboxMaps.MapboxMap) -> () in static rnmapbox_maps.RNMBXMapViewManager.getZoom(_: rnmapbox_maps.RNMBXMapView, resolver: (Swift.Optional<Any>) -> (), rejecter: (Swift.Optional<Swift.String>, Swift.Optional<Swift.String>, Swift.Optional<Swift.Error>) -> ()) -> () at <compiler-generated>, self=@thick rnmapbox_maps.RNMBXMapViewManager.Type) at RNMBXMapViewManager.swift:35:36
  frame #7: 0x0000000103fc9834 Seety`static RNMBXMapViewManager.getZoom(view=0x0000000127463be0, resolver=0x0000000103fcdca8 Seety`reabstraction thunk helper from @escaping @callee_unowned @convention(block) (@unowned Swift.Optional<Swift.AnyObject>) -> () to @escaping @callee_guaranteed (@in_guaranteed Swift.Optional<Any>) -> ()partial apply forwarder with unmangled suffix ".53" at <compiler-generated>, rejecter=0x0000000103fcdcec Seety`reabstraction thunk helper from @escaping @callee_unowned @convention(block) (@unowned Swift.Optional<__C.NSString>, @unowned Swift.Optional<__C.NSString>, @unowned Swift.Optional<__C.NSError>) -> () to @escaping @callee_guaranteed (@guaranteed Swift.Optional<Swift.String>, @guaranteed Swift.Optional<Swift.String>, @guaranteed Swift.Optional<Swift.Error>) -> ()partial apply forwarder with unmangled suffix ".57" at <compiler-generated>, self=@thick rnmapbox_maps.RNMBXMapViewManager.Type) at RNMBXMapViewManager.swift:127:13
  frame #8: 0x0000000103fc9a98 Seety`@objc static RNMBXMapViewManager.getZoom(_:resolver:rejecter:) at <compiler-generated>:0
  frame #9: 0x0000000103efc9bc Seety`__45-[RNMBXMapViewModule getZoom:resolve:reject:]_block_invoke(.block_descriptor=0x0000000282145350, view=0x0000000127463be0) at RNMBXMapViewModule.mm:97:9
  frame #10: 0x0000000103efb778 Seety`__58-[RNMBXMapViewModule withMapView:block:reject:methodName:]_block_invoke(.block_descriptor=0x00000002835ecec0, uiManager=0x00000002804fb100, viewRegistry=0x0000000282f9a220) at RNMBXMapViewModule.mm:42:12
  frame #11: 0x000000010364e6ac Seety`__44-[RCTUIManager flushUIBlocksWithCompletion:]_block_invoke(.block_descriptor=0x00000002821b4720) at RCTUIManager.m:1162:9
  frame #12: 0x000000010364e97c Seety`__44-[RCTUIManager flushUIBlocksWithCompletion:]_block_invoke.194(.block_descriptor=0x0000000283aaf240) at RCTUIManager.m:1182:5
  frame #13: 0x0000000103656408 Seety`__RCTExecuteOnMainQueue_block_invoke(.block_descriptor=0x00000002821b7c00) at RCTUtils.m:279:7
  frame #14: 0x000000010739c520 libdispatch.dylib`_dispatch_call_block_and_release + 32
  frame #15: 0x000000010739e038 libdispatch.dylib`_dispatch_client_callout + 20
  frame #16: 0x00000001073ae798 libdispatch.dylib`_dispatch_main_queue_drain + 1196
  frame #17: 0x00000001073ae2dc libdispatch.dylib`_dispatch_main_queue_callback_4CF + 44
  frame #18: 0x000000018b033c28 CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 16
  frame #19: 0x000000018b015560 CoreFoundation`__CFRunLoopRun + 1992
  frame #20: 0x000000018b01a3ec CoreFoundation`CFRunLoopRunSpecific + 612
  frame #21: 0x00000001c653035c GraphicsServices`GSEventRunModal + 164
  frame #22: 0x000000018d3a6f58 UIKitCore`-[UIApplication _run] + 888
  frame #23: 0x000000018d3a6bbc UIKitCore`UIApplicationMain + 340
  frame #24: 0x0000000102e6888c Seety`main(argc=2, argv=0x000000016cf9f7b8) at main.m:18:12
  frame #25: 0x00000001aa54cdec dyld`start + 2220

It's indeed in the accessor of mapboxMap for the getZoom() that the crashes occurs.

Hope it helps !

Gp2mv3 avatar May 06 '24 09:05 Gp2mv3

@Gp2mv3 thanks much awesome, this should be addressed in https://github.com/rnmapbox/maps/pull/3477 The difference between #3477 and this PR is that the former will execute the getZoom, getCenter, etc callbacks as soon as the mapView initialisation finished. While this PR just will not do some stuff until mapView has initialised.

mfazekas avatar May 07 '24 12:05 mfazekas