Use WeakGlobalRef for android MapController instance

- MapController is calling dispose, so it must be alive here
tangrams · Nov 30, 2018 · b4e5eff · b4e5eff
1 parent bf23539
commit b4e5eff
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 17 deletions.
diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp
@@ -181,7 +181,7 @@ std::string AndroidPlatform::fontPath(const std::string& _family, const std::str
 AndroidPlatform::AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance)
     : m_jniWorker(jvm) {
 
-    m_tangramInstance = _jniEnv->NewGlobalRef(_tangramInstance);
+    m_tangramInstance = _jniEnv->NewWeakGlobalRef(_tangramInstance);
 
     m_assetManager = AAssetManager_fromJava(_jniEnv, _assetManager);
 
@@ -193,10 +193,6 @@ AndroidPlatform::AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject
     sqlite3_ndk_init(m_assetManager);
 }
 
-void AndroidPlatform::dispose(JNIEnv* _jniEnv) {
-    _jniEnv->DeleteGlobalRef(m_tangramInstance);
-}
-
 void AndroidPlatform::requestRender() const {
     m_jniWorker.enqueue([&](JNIEnv *jniEnv) {
         jniEnv->CallVoidMethod(m_tangramInstance, requestRenderMethodID);

diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.h b/platforms/android/tangram/src/main/cpp/androidPlatform.h
@@ -31,7 +31,6 @@ class AndroidPlatform : public Platform {
 public:
 
     AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance);
-    void dispose(JNIEnv* _jniEnv);
     void shutdown() override {}
     void requestRender() const override;
     void setContinuousRendering(bool _isContinuous) override;

diff --git a/platforms/android/tangram/src/main/cpp/jniExports.cpp b/platforms/android/tangram/src/main/cpp/jniExports.cpp
@@ -144,16 +144,9 @@ extern "C" {
         return reinterpret_cast<jlong>(map);
     }
 
-    JNIEXPORT void JNICALL Java_com_mapzen_tangram_MapController_nativeDispose(JNIEnv* jniEnv, jobject obj, jlong mapPtr) {
+    JNIEXPORT void JNICALL Java_com_mapzen_tangram_MapController_nativeDispose(JNIEnv* jniEnv, jobject tangramInstance, jlong mapPtr) {
         assert(mapPtr > 0);
-        auto map = reinterpret_cast<Tangram::Map*>(mapPtr);
-        // Don't dispose MapController ref before map is teared down,
-        // delete map or worker threads might call back to it (e.g. requestRender)
-        auto platform = map->getPlatform();
-
-        delete map;
-
-        static_cast<Tangram::AndroidPlatform&>(*platform).dispose(jniEnv);
+        delete reinterpret_cast<Tangram::Map*>(mapPtr);
     }
 
     JNIEXPORT jint JNICALL Java_com_mapzen_tangram_MapController_nativeLoadScene(JNIEnv* jniEnv, jobject obj, jlong mapPtr, jstring path, jobjectArray updateStrings) {

diff --git a/platforms/android/tangram/src/main/java/com/mapzen/tangram/MapController.java b/platforms/android/tangram/src/main/java/com/mapzen/tangram/MapController.java
@@ -185,7 +185,7 @@ protected MapController(@NonNull Context context) {
         fontFileParser = new FontFileParser();
         fontFileParser.parse();
 
-        mapPointer = nativeInit(this, assetManager);
+        mapPointer = nativeInit(assetManager);
         if (mapPointer <= 0) {
             throw new RuntimeException("Unable to create a native Map object! There may be insufficient memory available.");
         }
@@ -1257,7 +1257,7 @@ boolean setMarkerDrawOrder(final long markerId, final int drawOrder) {
     // ==============
 
     private synchronized native void nativeOnLowMemory(long mapPtr);
-    private synchronized native long nativeInit(MapController instance, AssetManager assetManager);
+    private synchronized native long nativeInit(AssetManager assetManager);
     private synchronized native void nativeDispose(long mapPtr);
     private synchronized native int nativeLoadScene(long mapPtr, String path, String[] updateStrings);
     private synchronized native int nativeLoadSceneAsync(long mapPtr, String path, String[] updateStrings);