Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Don't call OnSurfaceCreated from the main thread #14244

Merged
merged 1 commit into from
Mar 27, 2019

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Mar 27, 2019

Follow up from #14127, closes #14224. We shouldn't call MapRenderer#OnSurfaceCreated from the SurfaceHolderCallback. This callback is invoked on the main thread and is invoked in pair with the already existing one on the render thread (this was a misassumption made in #14127 that this wasn't invoked in pairs).

This solves the root crash of

JNI DETECTED ERROR IN APPLICATION: thread Thread[1,tid=12787,Native,Thread*=0x77ea6c2a00,peer=0x73a481c8,"main"] using JNIEnv* from thread Thread[60,tid=12894,Runnable,Thread*=0x77cccb3000,peer=0x12dcaaa0,"GLThread 12903"]
2019-03-26 20:35:03.152 12787-12787/com.mapbox.mapboxsdk.testapp A/zygote64: java_vm_ext.cc:534] in call to DeleteGlobalRef

Which resulted in a bunch of undefined behavior crashes as #14224.

Have been running tests for hours and everything seems stable.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Mar 27, 2019
@tobrun tobrun added this to the release-liquid milestone Mar 27, 2019
@tobrun tobrun self-assigned this Mar 27, 2019
@tobrun tobrun requested a review from LukasPaczos March 27, 2019 08:40
Copy link
Contributor

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!

@@ -10,6 +10,7 @@ android {
versionCode 13
versionName "6.0.0"
testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"
//testInstrumentationRunnerArgument 'size', 'large'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually not, I took my a while to figure out this arg 🙃 so I was like, let's keep it there for future reference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to remove, we need command line support for this not a gradle configuration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@tobrun tobrun merged commit f07a24d into master Mar 27, 2019
@tobrun tobrun deleted the tvn-surface-created-main branch March 27, 2019 11:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Null Pointer Dereference Crash on CI
2 participants