Freezes when going back and forth between activites

[erf]

If you add a button to the main view in the test app, and open an arbitrary activity on a click event and then closes it, moves the map and click the button again, the app freezes on my nexus 7 (2012).
I made a simple test case here:
https://github.com/erf/mapbox-gl-native/tree/freezes

[erf]

This also seem to happen if i moved the map first and then click on a button which opens the new activity. I don't see any information in the Log regarding this. I suspected this may be a memory issue when opening the map several times, so i even tried to make the mapview static, but with same result. I suspect now that some state is getting set when you move the map, but i have no idea what causes the crash.

[erf]

I just made a simple example now on latest 4025da6 and this seem to work now! I close this for now.

[erf]

This issue is back after i added loading mapView as a fragment described in #2042

[tobrun]

Was able to reproduce this occasionally with following setup:
Genymotion - Google Nexus 5.0.0 image - 0.0.1-SNAPSHOT

Actions:
Open activity with MapView, scroll-zoom actions like a maniac, open other Activity containing a MapView. Application becomes unresponsive and screens turns black. Only option is to kill-app by swiping it from recents.

Used Activities:
StyledMapActivity - SearchMapActivity

Full stacktrace - code

[erf]

I think the app crashes when trying to open a new activity whilst the MapView is still loading tiles.

Threading issue?

[ljbade]

I also wonder if it is related to state saving.

[ljbade]

@kkaefer Is there anything in the C++ layer to be aware of when there are multiple instances of a map?

[kkaefer]

@ljbade it should be possible and I'm not aware of an issues, but of course there could be bugs when using multiple instances.

[incanus]

We are handling multiple maps in iOS ok right now.

[ljbade]

Hmm going to have to create a simple test app to see if I can reproduce it.

@incanus Do you recall if there were any specific fixes/changes that perhaps did not make it to Android to enable multiple maps?

[tobrun]

@ljbade a simple test app can be found here

[ljbade]

Thanks @tobrunvannuland

[erf]

Simple test case:
Try to have a map in both activity A and B. Open activity B from A and click in the map before all the tiles are loaded.

[ljbade]

I done some reading at it appears the Android system does not support two SurfaceViews active at the same time.

So this bug might be due to limitations as a result of using SurfaceView.

Perhaps we will need to switch to TextureView to support more complex layouts.

[ljbade]

We are pushing TextureView to 0.2.0 so we should wait to fix this bug then?

@bleege ?

[bleege]

Perhaps we will need to switch to TextureView to support more complex layouts.

@ljbade Thanks for looking into this. Before we can make a decision on milestone can you provide more detail as to why you believe TextureView would solve this issue? If there's a firm reason why you believe it should then we can definitely look to include #2244 in 0.1.0.

[ljbade]

@bleege Basically we use SurfaceView. It is well documented the Android system does not support two SurfaceViews on the screen at the same time. This would include two different Activitiy where there might be some sort of transition or delay between the new MapView being created and the old MapView getting deleted.

TextureView does not have this limitation.

[ljbade]

Although we also need to test if TextureView will fix this bug, in case it is something related to the native layer.

[ljbade]

@erf Do you mind recreating the bug against the TextureView implementation in https://github.com/mapbox/mapbox-gl-native/tree/2244-texture-view branch?

I would like to know if this is a solution.

[erf]

Will do. Just really busy right now..

[ljbade]

@bleege Can you give https://github.com/mapbox/mapbox-gl-native/tree/2018-shared-sqlite a test drive?

Testing on the Samsung S6 I don't get the database lock errors. But I still get the freeze on rotation and the occasional crash.

[erf]

Exited for this progress! Cheering =)

[kkaefer]

@ljbade can we use a uv_once/pthread_once?

[kkaefer]

Actually never mind, the existing code looks good.

[zugaldia]

I'm adding a SecondMapActivityTest class to the Espresso test suite to help troubleshoot this.

[bleege]

I'm looking into the 2018-shared-sqlite branch, but ran into an issue with make android now from #2288. I'm going to undo it in this branch to test it out.

[bleege]

Ok, I've done my time in C++ land and can test this out now.

[bleege]

\o/ The database lock errors are now gone. I was able to rotate the test device from portrait to landscape and back again several times without any problems.! Great job @ljbade!

Will discuss the NPM issue with @lucaswoj before merging in though. Hopefully later today.

[incanus]

To round this out, we should probably get rid of iOS's MGLFileCache and move to this lower-level C++ implementation. I can look at that.

[incanus]

To be clear: it's not a blocker and we can ticket off the iOS refactor separately.

[bleege]

The plan is to remove 3764b051e4c9944ddbc3bcc9c8fd1beb8a28451e from 2018-shared-sqlite and then merge @ljbade's fix into master. We can then cherrypick that commit into release-android-0.1.0 so that both master and release-android-0.1.0 have it and a SNAPSHOT can be rolled because release-android-0.1.0 does not have the geojsonvt stuff.

Concurrently @lucaswoj @mikemorris will work on getting geojsonvt working again so that regular Android development can continue on master.

/cc @jfirebaugh

[incanus]

Refs #2344 re: geojsonvt problems introduced.

[bleege]

Merged. Now the cherrypick to release-android-0.1.0.

[incanus]

Speculatively fixed in #2345.

[erf]

Awsome work !! I would like to test this out but i get the following build error:

deps/run_gyp geojsonvt.gyp -Iconfig.gypi --depth=. -Goutput_dir=. --generator-output=./build -f make
make -C build install
  CXX(target) Release/obj.target/geojsonvt/src/geojsonvt.o
  CXX(target) Release/obj.target/geojsonvt/src/geojsonvt_clip.o
  CXX(target) Release/obj.target/geojsonvt/src/geojsonvt_convert.o
  CXX(target) Release/obj.target/geojsonvt/src/geojsonvt_simplify.o
  CXX(target) Release/obj.target/geojsonvt/src/geojsonvt_tile.o
  LIBTOOL-STATIC Release/libgeojsonvt.a
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: file: Release/obj.target/geojsonvt/src/geojsonvt.o is not an object file (not allowed in a library)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: file: Release/obj.target/geojsonvt/src/geojsonvt_clip.o is not an object file (not allowed in a library)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: file: Release/obj.target/geojsonvt/src/geojsonvt_convert.o is not an object file (not allowed in a library)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: file: Release/obj.target/geojsonvt/src/geojsonvt_simplify.o is not an object file (not allowed in a library)
error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: file: Release/obj.target/geojsonvt/src/geojsonvt_tile.o is not an object file (not allowed in a library)
make[3]: *** [Release/libgeojsonvt.a] Error 1
make[2]: *** [install] Error 2
make[1]: *** [config/android-arm-v7.gypi] Error 2
make: *** [android-lib] Error 2

[incanus]

@erf We are tracking that in #2348. For now you can test out the release-android-v0.1.0 branch, where we cherry-picked the fix, to essentially be based off of an older version of master that builds fine.

[erf]

Ok, thanks!

[erf]

i checked out the 'release-android-v0.1.0' branch, built it and tried it with my test app here: https://github.com/erf/mapbox-drawer-glitch/tree/open-activity-whilst-scrolling
It seems to work much better now, however i experienced a crash after going back and forth between the activites a few times, in the log i only got these:

09-17 12:21:23.961  32460-32671/? A/libc﹕ Fatal signal 11 (SIGSEGV), code 1, fault addr 0x3c in tid 32671 (sm-v1/2/2/1.pbf)
09-17 12:28:04.231    3204-3483/? A/libc﹕ Fatal signal 11 (SIGSEGV), code 1, fault addr 0x3c in tid 3483 (-v1/6/33/16.pbf)