Workaround for disposal crash with flutter3

[srmncnk]

This PR addresses native crashes after Mapbox disposal, since flutter3 seems to dispose resources faster than its 2.X.X predecessors.

It is an ugly workaround and by no means a fix, so be advised to use it cautiously.

It works for both hybrid and virtual device mode.

[felix-ht]

Nice job!

Can you only use the AndroidViewWithWrappedController if we are on flutter 3.x and the old if not?

This avoids any risk for old versions!

[srmncnk]

I don't think there's a way to get flutter version runtime, only dart. I suppose we could introduce a breaking change with a parameter and let the user decice, which kind of embedding they want?

[felix-ht]

huh i didn’t know that you could not get the flutter version.

So adding a parameter would be a good solution. We can start by defaulting it to the current embedding

Adding some lines to the README to indicate that this should be set someone wants to use flutter 3.x, would be great. Also clearly state that this is a workaround and should be used with care.

[srmncnk]

I'll add the parameter, the corresponding warning and fix the failed checks. I can also update the README and you can later polish it up.

[srmncnk]

I added two parameters. One for delayed disposal and one for overriding hybrid composition. Both act per instance of map.

Side note: In the project I'm working on we've come to the conclusion that hybrid mode is better (faster and without crashes) for Android 9 devices since we manufacture one - that contradicts observations that were commented in this repo. Hence I added an override parameter, so that you can have a mixed mode and decide whatever you want - run time.

Aside from this there's not much I can do - I tested your proposed solution and commented in https://github.com/flutter-mapbox-gl/maps/issues/1119. If you have any more suggestions, please let me know. Otherwise please review this and consider merging.

[samderlust]

@srmanc I try to add this branch into my project by using this

  mapbox_gl:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac

but I got the error

: Error: Type 'SnapshotOptions' not found.
../…/src/controller.dart:1252
  Future<String> takeSnapshot(SnapshotOptions snapshotOptions) async {
                              ^^^^^^^^^^^^^^^
: Error: 'SnapshotOptions' isn't a type.
../…/src/controller.dart:1252
  Future<String> takeSnapshot(SnapshotOptions snapshotOptions) async {
                              ^^^^^^^^^^^^^^^
: Error: The method 'takeSnapshot' isn't defined for the class 'MapboxGlPlatform'.
../…/src/controller.dart:1253
- 'MapboxGlPlatform' is from 'package:mapbox_gl_platform_interface/mapbox_gl_platform_interface.dart' ('../../../.pub-cache/git/flutter-mapbox-gl-3496907955cd4b442e4eb905d67e8d46692174f1/mapbox_gl_platform_interface/lib/mapbox_gl_platform_interface.dart').
package:mapbox_gl_platform_interface/mapbox_gl_platform_interface.dart:1
Try correcting the name to the name of an existing method, or defining a method named 'takeSnapshot'.
    return _mapboxGlPlatform.takeSnapshot(snapshotOptions);
                             ^^^^^^^^^^^^

You have any idea?

[srmncnk]

@samderlust Didn't try it out, but AFAIK you have to make dependency overrides for mapbox_gl_platform_interface and mapbox_gl_web as well, not just mapbox_gl.

[samderlust]

@srmanc I figure so, let me try that THanks

[samderlust]

it is compiled with the following. just put it here in case anyone needs it

dependency_overrides:
  mapbox_gl:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
  mapbox_gl_platform_interface:
    git:
      url: https://github.com/tobrun/flutter-mapbox-gl.git
      ref: master
      path: mapbox_gl_platform_interface
  mapbox_gl_web:
    git:
      url: https://github.com/tobrun/flutter-mapbox-gl.git
      ref: master
      path: mapbox_gl_web

[srmncnk]

@samderlust please note that Tobrun's repository is deprecated, https://github.com/flutter-mapbox-gl/maps.git is the official fork we all should be using. If you want to use this PR, I suggest to use the same commit for all three dependencies, since they all come from the same repo.

Like this:

dependency_overrides:
  mapbox_gl:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
  mapbox_gl_platform_interface:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
      path: mapbox_gl_platform_interface
  mapbox_gl_web:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
      path: mapbox_gl_web

[samderlust]

@srmanc Thanks for correcting.

[dawid-niedzwiecki]

I'm having an issue while setting up my project to use this PR. I tried setting my pubspec.yaml up just like @srmanc stated. Though I'm getting an error while compiling:

../../.pub-cache/git/maps-7eead7c3fa4b59f553ee15a33ab12e8c564e60ac/mapbox_gl_platform_interface/lib/src/view_wrappers.dart:29:7: Error: The non-abstract class 'TextureAndroidViewControllerWrapper' is missing implementations for these members:
 - TextureAndroidViewController.requiresViewComposition
Try to either
 - provide an implementation,
 - inherit an implementation from a superclass or mixin,
 - mark the class as abstract, or
 - provide a 'noSuchMethod' implementation.

class TextureAndroidViewControllerWrapper
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
../../flutter/packages/flutter/lib/src/services/platform_views.dart:1155:12: Context: 'TextureAndroidViewController.requiresViewComposition' is defined here.
  bool get requiresViewComposition {
           ^^^^^^^^^^^^^^^^^^^^^^^

This is my pubspec.yaml:

name: my_app
description: MyApp

publish_to: 'none'

version: 2.0.0+65

environment:
  sdk: ">=2.15.1 <3.0.0"

dependencies:
  jwt_decode: ^0.3.1
  mapbox_gl:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac

dependency_overrides:
  mapbox_gl:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
  mapbox_gl_platform_interface:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
      path: mapbox_gl_platform_interface
  mapbox_gl_web:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
      path: mapbox_gl_web

dev_dependencies:
  flutter_launcher_icons: "^0.9.2"
  flutter_lints: ^1.0.0
  flutter_test:
    sdk: flutter
  lints: ^1.0.1

Could you provide me with some feedback what I'm doing wrong?

[srmncnk]

@dawid-niedzwiecki This PR works with the latest stable flutter, 3.3.0. Which version of flutter are you using?

[dawid-niedzwiecki]

@Edit:

It works <3 Had to use flutter 3.3.0 and set my MapboxMap properties like this:

useHybridCompositionOverride: true,
useDelayedDisposal: true,

[GULERTOLGA]

Unfortunately I'm still having the same problem

[✓] Flutter (Channel stable, 3.3.3, on macOS 12.0.1 21A559 darwin-x64, locale en-TR) [✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0) [!] Xcode - develop for iOS and macOS (Xcode 13.0) ! CocoaPods 1.10.1 out of date (1.11.0 is recommended). CocoaPods is used to retrieve the iOS and macOS platform side's plugin code that responds to your plugin usage on the Dart side. Without CocoaPods, plugins will not work on iOS or macOS. For more info, see https://flutter.dev/platform-plugins To upgrade see https://guides.cocoapods.org/using/getting-started.html#installation for instructions. [✓] Chrome - develop for the web [✓] Android Studio (version 2021.1) [✓] VS Code (version 1.71.2) [✓] Connected device (3 available) [✓] HTTP Host Availability

Build fingerprint: 'Xiaomi/laurel_sprout/laurel_sprout:11/RKQ1.200903.002/V12.0.14.0.RFQMIXM:user/release-keys'
Revision: '0'
ABI: 'arm64'
Timestamp: 2022-10-04 14:49:34+0300
pid: 24343, tid: 24778, name: GLThread 5435  >>> com.netcad.netigma3 <<<
uid: 10264
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
Cause: null pointer dereference
    x0  0000000000000000  x1  0000000000000000  x2  0000000000000001  x3  0000007bd5203878
    x4  0000007b46bf5e58  x5  000000000000004a  x6  ff284900ff302349  x7  7f7f7f7f7f7f7f7f
    x8  0000000000000000  x9  0000000000000001  x10 0000000000000001  x11 0000000000000000
    x12 0000007b46bf5fe4  x13 0000007b46bf5fd8  x14 0000000000000000  x15 00000000ebad6a89
    x16 0000007b1b4c0f70  x17 0000007eca13281c  x18 00000077ee64e000  x19 0000007b46bf5f78
    x20 0000000000000000  x21 0000000000000001  x22 0000000012f00d88  x23 0000000014285470
    x24 00005b22b78d8c22  x25 0000000000000001  x26 0000000000000000  x27 0000000000000000
    x28 0000007b46bf5ff0  x29 0000007b46bf5f10
    lr  0000007b1b0ab81c  sp  0000007b46bf5f00  pc  0000007b1b0ab948  pst 0000000060000000
backtrace:
      #00 pc 000000000011d948  /data/app/~~oMSB0aZJ_DcREdEyxFhUQg==/com.netcad.netigma3-czg7_E9tnckySgdXRq9EXA==/base.apk!libmapbox-gl.so (offset 0x28f5000) (BuildId: 5340fab4c92fd87f8304775de4bc95627e863fd7)
      #01 pc 000000000011d818  /data/app/~~oMSB0aZJ_DcREdEyxFhUQg==/com.netcad.netigma3-czg7_E9tnckySgdXRq9EXA==/base.apk!libmapbox-gl.so (offset 0x28f5000) (BuildId: 5340fab4c92fd87f8304775de4bc95627e863fd7)
      #02 pc 00000000000adc2c  /data/app/~~oMSB0aZJ_DcREdEyxFhUQg==/com.netcad.netigma3-czg7_E9tnckySgdXRq9EXA==/base.apk!libmapbox-gl.so (offset 0x28f5000) (BuildId: 5340fab4c92fd87f8304775de4bc95627e863fd7)
      #03 pc 000000000013ced4  /apex/com.android.art/lib64/libart.so (art_quick_generic_jni_trampoline+148) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #04 pc 000000000204ca74  /memfd:jit-cache (deleted) (offset 0x2000000) (com.mapbox.mapboxsdk.maps.renderer.MapRenderer.onDrawFrame+100)
      #05 pc 00000000020548fc  /memfd:jit-cache (deleted) (offset 0x2000000) (com.mapbox.mapboxsdk.maps.renderer.glsurfaceview.GLSurfaceViewMapRenderer.onDrawFrame+44)
      #06 pc 0000000002033f90  /memfd:jit-cache (deleted) (offset 0x2000000) (com.mapbox.mapboxsdk.maps.renderer.glsurfaceview.MapboxGLSurfaceView$GLThread.guardedRun+2960)
      #07 pc 0000000000133564  /apex/com.android.art/lib64/libart.so (art_quick_invoke_stub+548) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #08 pc 00000000001a8a78  /apex/com.android.art/lib64/libart.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+200) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #09 pc 000000000031831c  /apex/com.android.art/lib64/libart.so (art::interpreter::ArtInterpreterToCompiledCodeBridge(art::Thread*, art::ArtMethod*, art::ShadowFrame*, unsigned short, art::JValue*)+376) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #10 pc 000000000030e648  /apex/com.android.art/lib64/libart.so (bool art::interpreter::DoCall<false, false>(art::ArtMethod*, art::Thread*, art::ShadowFrame&, art::Instruction const*, unsigned short, art::JValue*)+996) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #11 pc 000000000067ebc0  /apex/com.android.art/lib64/libart.so (MterpInvokeDirect+576) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #12 pc 000000000012d914  /apex/com.android.art/lib64/libart.so (mterp_op_invoke_direct+20) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #13 pc 00000000000e8458  [anon:dalvik-classes7.dex extracted in memory from /data/app/~~oMSB0aZJ_DcREdEyxFhUQg==/com.netcad.netigma3-czg7_E9tnckySgdXRq9EXA==/base.apk!classes7.dex] (com.mapbox.mapboxsdk.maps.renderer.glsurfaceview.MapboxGLSurfaceView$GLThread.run+48)
      #14 pc 0000000000305c44  /apex/com.android.art/lib64/libart.so (art::interpreter::Execute(art::Thread*, art::CodeItemDataAccessor const&, art::ShadowFrame&, art::JValue, bool, bool) (.llvm.11487796752256266877)+268) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #15 pc 000000000066b24c  /apex/com.android.art/lib64/libart.so (artQuickToInterpreterBridge+780) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #16 pc 000000000013cff8  /apex/com.android.art/lib64/libart.so (art_quick_to_interpreter_bridge+88) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #17 pc 0000000000133564  /apex/com.android.art/lib64/libart.so (art_quick_invoke_stub+548) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #18 pc 00000000001a8a78  /apex/com.android.art/lib64/libart.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+200) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #19 pc 0000000000554cbc  /apex/com.android.art/lib64/libart.so (art::JValue art::InvokeVirtualOrInterfaceWithJValues<art::ArtMethod*>(art::ScopedObjectAccessAlreadyRunnable const&, _jobject*, art::ArtMethod*, jvalue const*)+460) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #20 pc 00000000005a4058  /apex/com.android.art/lib64/libart.so (art::Thread::CreateCallback(void*)+1308) (BuildId: c58eb791dd4db5a5fc1ef4a3404cd082)
      #21 pc 00000000000b0048  /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+64) (BuildId: 8d77279a411c99f8bc6edb79c76340fb)
      #22 pc 00000000000503c8  /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64) (BuildId: 8d77279a411c99f8bc6edb79c76340fb)
Lost connection to device.

[srmncnk]

@GULERTOLGA Did you use useDelayedDisposal: true? What steps did you use exactly to reproduce this issue?

[GULERTOLGA]

Yes I have set useDelayedDisposal=true. There seems to be no definite reproducing steps. I often encounter this error when I go back to the previous screen after pan and zoom operations on the map.

my pubspec

environment:
  sdk: '>=2.18.2 <3.0.0'

dependencies:
  mapbox_gl:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac

dependency_overrides:
  test_api: ^0.4.14
  mapbox_gl:
    git:
      url: https://github.com/srmanc/maps.git
      ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
  mapbox_gl_platform_interface:
      git:
        url: https://github.com/srmanc/maps.git
        ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
        path: mapbox_gl_platform_interface
  mapbox_gl_web:
      git:
        url: https://github.com/srmanc/maps.git
        ref: 7eead7c3fa4b59f553ee15a33ab12e8c564e60ac
        path: mapbox_gl_web

[srmncnk]

@GULERTOLGA Did you reproduce this on this repo's example or in your own implementation? This PR basically delays the disposal of the native screen - so, if you by any chance manipulate Mapbox after it has been (or seems to be) disposed, the crash happens.

[GULERTOLGA]

[dawid-niedzwiecki]

This PR managed to solve my issue of the app crashing when removing it from the stack, although I now experience a different problem. When using the map, locking the screen and coming back to the app, going to background and switching back to foreground, I see a lot of dequeueBuffer: Buffer has been abandoned errors. Thousands actually. That seems to drop performance of the map substantially.

[srmncnk]

@GULERTOLGA Thanks for the screen grab. Is the crash and querying the map coincidental or does the crash happen every time you query the map?

[srmncnk]

@dawid-niedzwiecki I'm seeing these messages at my end as well, I don't experience a performance drop. Not much I can do from here at this point.

[dawid-niedzwiecki]

@dawid-niedzwiecki I'm seeing these messages at my end as well, I don't experience a performance drop. Not much I can do from here at this point.

Well, maybe I'm misinterpreting it and it's not the cause of the performance drop. I appreciate your work, finally I can finish my app.

[GULERTOLGA]

@GULERTOLGA Thanks for the screen grab. Is the crash and querying the map coincidental or does the crash happen every time you query the map?

The error can repeat even if the map is only open for a while. I also debugged the Android project of the application, but I couldn't find anything useful. Maybe it's something with my configuration. That's why I'm going to do a test with the official example application and let you know.

[tanphuccgl]

I'm still having the same problem. I used useDelayedDisposal: true and update dependency_overrides with version 3.3.0 . Is there anything new that's better? 🙁

[srmncnk]

@GULERTOLGA @tanphuccgl are you able to preproduce this with this repo's example? Can you also please try this with useHybridCompositionOverride: true? From my experience I experienced similar crashes with virtual device mode (the opposite of hybrid composition) enabled, though it was hard to reproduce and it happened only after a while and moderate / heavy load.

[srmncnk]

This PR seems to fix the issue for some users while some still experience the same issue but haven't responded all inquiries. Do you @felix-ht think we could merge this since it's opt-in anyway or do you think we could somehow improve this further?

[felix-ht]

@srmanc my only concern is that this might indicate to the users that it works while it really for lots of users right now. If you can extend the message to indicate that this is not ready for production and has some residual issues - im fine with merging it.

your take @AAverin ?

[srmncnk]

@felix-ht Happy to do it, see https://github.com/flutter-mapbox-gl/maps/pull/1172/commits/63b3d71e37a01269432e766b3cacec60c8e7bcdb.

[AAverin]

I am personally reluctant to merge. Anyone adventurous enough can use the branch directly via dependency_overrides and for everyone else having an opt-in flag will introduce a false safety for Flutter 3.x while not fixing the underlying issue.

Unfortunately I didn't yet have time to look deeper into the issue myself. Too much work.

[felix-ht]

@AAverin i do agree that this isn't perfect - but the thing is that with the addition into the readme is certainly better than the current state of master.

It has a clear indication what to expect with flutter 3.X and makes it easier for users to start testing, potentially giving us more awareness for the flutter and upstream mapbox issues.

[felix-ht]

@srmanc can you fix Flutter CI / Static code analysis (pull_request)

[AAverin]

I can see all the valid points so I am fine with merging.

Although, as this is not a fix, I think a better way would be to keep this workaround as a separate branch and add to main readme instructions on how people can use it via dependency_overrides and test if they really really need to migrate to Flutter 3 and are ready to face all the consequences of a workaround.

[srmncnk]

@felix-ht Locally for me it works, I'm using flutter 3.3.3. Linter says it's missing Uint8List which is in dart:typed_data, but my local linter says it's also flutter/services package. Which flutter does CI use?

@AAverin I agree and I see your point, it's not a fix. One argument though for why we benefit from this - flutter web has a lot of improvements and bug fixes after 3.x and there are users, who use this package cross-platform completely - myself included.

[felix-ht]

The Ci is on 2.10 - (which is still the main version used for the package)

[felix-ht]

@AAverin i'am not in favor the extra branch, as this will cause additional maintenance effort to keep this branch in sync with master. And we are not talking about a release but just the current master.

[srmncnk]

@felix-ht Linter errors and formatting fixed.

[felix-ht]

@srmanc i decided to revert this merge commit as it causes issues with old flutter versions - apparently the interface of TextureAndroidViewController changed - this causes to no longer build for flutter 2.X

Might be quite tricky to get this working for 2.x and 3.x

[srmncnk]

@felix-ht consider https://github.com/flutter-mapbox-gl/maps/pull/1212. It's a bit lucky, but reflection wouldn't work anyway. Compiles and seems to work.