mapbox / mapbox-maps-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and Metal
https://www.mapbox.com/mapbox-mobile-sdk
Other
462 stars 150 forks source link

Snapshotter init deadlocks waiting for TelemetryService destructor #1760

Open jameshurst opened 1 year ago

jameshurst commented 1 year ago

Environment

Observed behavior and steps to reproduce

When initializing a Snapshotter the main thread gets locked up waiting for the TelemetryService destructor.

This issue still occurs when using mapbox-events-ios v1.0.10.

Screen Shot 2022-11-28 at 6 30 35 PM
ZiZasaurus commented 1 year ago

@jameshurst thank you for reporting. The team is investigating and as soon as I have an update, I will provide it here.

jing-yu-1 commented 1 year ago

I've encountered the same locking issue. With some investigation, we found that if we try to initialize a Snapshotter before a MapView has been initialized, the Snapshotter initializer will hang forever. However, if a mapview has been initialized somewhere, there is no problem initializing the Snapshotter. We also saw that this deadlock exists in Mapbox v10.10.1, but not in v10.9.

vladpre92 commented 1 year ago

My init code is:

let options = MapSnapshotOptions(size: snapshotSize, pixelRatio: pixelRatio, showsLogo: false, showsAttribution: false)
self.snapshotter = .init(options: options)

Nothing special.

I guess it's a race condition somewhere. As @jing-yu-1 said, if we have a MapView initialized somewhere, this deadlock won't happen (I also checked this scenario).

In my case I get the deadlock only on simulators, and only if I present the screen that initializes the Snapshotter by long press and release a UITableViewCell. I think same will happen if I change it to a UIButton long press. If I make a regular tap like press and immediately release, then everything works fine.

Hope this will help with the debug

vladpre92 commented 1 year ago

@ZiZasaurus Hi there! Do we have any update on this issue? I think you did some fixes on the Snapshotter with v10.11.0, and I noticed that some things changed. Now, if we don't create an instance of a MapView before creating a Snapshotter the app will directly crash instead of deadlock. It's very frustrating because there are a lot of use cases when you would want to generate a snapshot without a MapView, and creating one just for snapshots would mean to use a lot of resources.

Here is the crash log crash.ips.zip

Thank you!

jing-yu-1 commented 1 year ago

We also tried v10.11.1, we got crash in Snapshotter too.

Crashed: com.mapbox.common.Snapshotter
EXC_BAD_ACCESS KERN_INVALID_ADDRESS 0x0000000000000000
0
Metal
-[_MTLLibrary newFunctionWithName:constantValues:functionCache:specializedName:error:] + 340
1
MapboxCoreMaps
mbgl::metal::createRenderPipeline(mbgl::metal::Context&, char const*, unsigned char, std::__1::vector<std::experimental::optional<mbgl::gfx::AttributeBinding>, std::__1::allocator<std::experimental::optional<mbgl::gfx::AttributeBinding> > > const&, mbgl::metal::AttachmentPixelFormats const&, mbgl::gfx::ColorMode const&, unsigned int, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) + 1020
2
MapboxCoreMaps
mbgl::metal::Program<mbgl::BackgroundProgram>::draw(mbgl::gfx::Context&, mbgl::gfx::RenderPass&, mbgl::gfx::RenderMode, mbgl::gfx::DrawMode const&, mbgl::gfx::DepthMode const&, mbgl::gfx::StencilMode const&, mbgl::gfx::ColorMode const&, mbgl::gfx::CullFaceMode const&, mbgl::gfx::UniformValues<mbgl::TypeList<mbgl::uniforms::matrix, mbgl::uniforms::color, mbgl::uniforms::opacity> > const&, mbgl::gfx::DrawScope&, mbgl::gfx::AttributeBindings<mbgl::TypeList<mbgl::attributes::pos> > const&, mbgl::gfx::TextureBindings<mbgl::TypeList<> > const&, mbgl::gfx::IndexBuffer const&, unsigned long, unsigned long) + 772
3
MapboxCoreMaps
mbgl::RenderBackgroundLayer::render(mbgl::PaintParameters&) + 2764
4
MapboxCoreMaps
mbgl::(anonymous namespace)::LayerRenderItemImpl::render(mbgl::PaintParameters&) const + 92
5
MapboxCoreMaps
mbgl::Renderer::Impl::render(mbgl::RenderTree const&) + 15972
6
MapboxCoreMaps
mbgl::Renderer::render(std::__1::shared_ptr<mbgl::UpdateParameters> const&) + 596
7
MapboxCoreMaps
std::__1::__function::__func<mbgl::SnapshotterFrontend::update(std::__1::shared_ptr<mbgl::UpdateParameters>)::$_0, std::__1::allocator<mbgl::SnapshotterFrontend::update(std::__1::shared_ptr<mbgl::UpdateParameters>)::$_0>, void ()>::operator()() + 404
8
MapboxCommon
void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, mapbox::common::ThreadedSchedulerBase::makeSchedulerThread(unsigned long, mapbox::base::WeakPtr<mapbox::common::Scheduler>, mapbox::common::ThreadServiceType)::$_0> >(void*) + 828
9
libsystem_pthread.dylib
_pthread_start + 148
10
libsystem_pthread.dylib
thread_start + 8
OdNairy commented 1 year ago

We cannot reproduce the issue in the latest main branch. May I ask you to try it out and if it's still here we will need some sample app.

vladpre92 commented 1 year ago

Hi @OdNairy !

I can confirm the fix. We cannot reproduce it anymore starting with https://github.com/mapbox/mapbox-maps-ios/releases/tag/v10.11.0.

The crash that I mentioned above was misinterpreted because of the still open Snapshotter issue that I can reproduce even with the latest version: https://github.com/mapbox/mapbox-maps-ios/issues/1928.

Do you have any E.T.A on that one?

Thank you!