From 3bcd9d6b2f586f153c75309b19b6835ffb7b6aca Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Fri, 16 Nov 2018 16:27:03 +0100 Subject: [PATCH 1/6] Add Platform::shutdown() to stop workers before going to delete Map --- core/include/tangram/platform.h | 1 + .../tangram/src/main/cpp/androidPlatform.h | 1 + platforms/ios/framework/src/iosPlatform.h | 1 + platforms/linux/src/linuxPlatform.cpp | 49 ++++++++++++------- platforms/linux/src/linuxPlatform.h | 13 ++--- platforms/osx/src/osxPlatform.h | 1 + tests/src/mockPlatform.h | 1 + 7 files changed, 42 insertions(+), 25 deletions(-) diff --git a/core/include/tangram/platform.h b/core/include/tangram/platform.h index 354ebac785..163b59060e 100644 --- a/core/include/tangram/platform.h +++ b/core/include/tangram/platform.h @@ -61,6 +61,7 @@ class Platform { Platform(); virtual ~Platform(); + virtual void shutdown() = 0; // Request that a new frame be rendered by the windowing system virtual void requestRender() const = 0; diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.h b/platforms/android/tangram/src/main/cpp/androidPlatform.h index 826afa97fa..5e914081de 100644 --- a/platforms/android/tangram/src/main/cpp/androidPlatform.h +++ b/platforms/android/tangram/src/main/cpp/androidPlatform.h @@ -30,6 +30,7 @@ class AndroidPlatform : public Platform { AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance); void dispose(JNIEnv* _jniEnv); + void shutdown() override {} void requestRender() const override; void setContinuousRendering(bool _isContinuous) override; FontSourceHandle systemFont(const std::string& _name, const std::string& _weight, const std::string& _face) const override; diff --git a/platforms/ios/framework/src/iosPlatform.h b/platforms/ios/framework/src/iosPlatform.h index 521f622dec..42a13e26c0 100644 --- a/platforms/ios/framework/src/iosPlatform.h +++ b/platforms/ios/framework/src/iosPlatform.h @@ -11,6 +11,7 @@ class iOSPlatform : public Platform { public: iOSPlatform(__weak TGMapView* _mapView); + void shutdown() override {} void requestRender() const override; void setContinuousRendering(bool _isContinuous) override; std::vector systemFontFallbacksHandle() const override; diff --git a/platforms/linux/src/linuxPlatform.cpp b/platforms/linux/src/linuxPlatform.cpp index 200a4d5925..3c305bb96c 100644 --- a/platforms/linux/src/linuxPlatform.cpp +++ b/platforms/linux/src/linuxPlatform.cpp @@ -26,41 +26,52 @@ void logMsg(const char* fmt, ...) { } LinuxPlatform::LinuxPlatform() : - m_urlClient(UrlClient::Options{}) { + m_urlClient(std::make_unique(UrlClient::Options{})) { m_fcConfig = FcInitLoadConfigAndFonts(); } LinuxPlatform::LinuxPlatform(UrlClient::Options urlClientOptions) : - m_urlClient(urlClientOptions) {} + m_urlClient(std::make_unique(urlClientOptions)) { + m_fcConfig = FcInitLoadConfigAndFonts(); +} + +LinuxPlatform::~LinuxPlatform() { + FcConfigDestroy(m_fcConfig); +} + +void LinuxPlatform::shutdown() { + // Stop all UrlWorker threads + m_shutdown = true; + m_urlClient.reset(); +} void LinuxPlatform::requestRender() const { + if (m_shutdown) { return; } glfwPostEmptyEvent(); } std::vector LinuxPlatform::systemFontFallbacksHandle() const { - /* - * Read system fontconfig to get list of fallback font for each supported language - */ + // Read system fontconfig to get list of fallback font for each + // supported language auto fallbackFonts = systemFallbackFonts(m_fcConfig); - /* - * create FontSourceHandle from the found list of fallback fonts - */ + // Create FontSourceHandle from the found list of fallback fonts std::vector handles; handles.reserve(fallbackFonts.size()); - for (auto& path : fallbackFonts) { - handles.emplace_back(Url(path)); - } + std::transform(std::begin(fallbackFonts), std::end(fallbackFonts), + std::back_inserter(handles), + [](auto& path) { return FontSourceHandle(Url(path)); }); return handles; } -FontSourceHandle LinuxPlatform::systemFont(const std::string& _name, const std::string& _weight, +FontSourceHandle LinuxPlatform::systemFont(const std::string& _name, + const std::string& _weight, const std::string& _face) const { - std::string fontFile = systemFontPath(m_fcConfig, _name, _weight, _face); + auto fontFile = systemFontPath(m_fcConfig, _name, _weight, _face); if (fontFile.empty()) { return {}; } @@ -68,15 +79,19 @@ FontSourceHandle LinuxPlatform::systemFont(const std::string& _name, const std:: } UrlRequestHandle LinuxPlatform::startUrlRequest(Url _url, UrlCallback _callback) { - return m_urlClient.addRequest(_url.string(), _callback); + if (m_shutdown) { return 0; } + return m_urlClient->addRequest(_url.string(), + [this, cb = _callback](UrlResponse&& r) { + cb(std::move(r)); + requestRender(); + }); } void LinuxPlatform::cancelUrlRequest(UrlRequestHandle _request) { - m_urlClient.cancelRequest(_request); + if (m_shutdown) { return; } + m_urlClient->cancelRequest(_request); } -LinuxPlatform::~LinuxPlatform() {} - void setCurrentThreadPriority(int priority) { setpriority(PRIO_PROCESS, 0, priority); } diff --git a/platforms/linux/src/linuxPlatform.h b/platforms/linux/src/linuxPlatform.h index 27e5b83330..fc482c031f 100644 --- a/platforms/linux/src/linuxPlatform.h +++ b/platforms/linux/src/linuxPlatform.h @@ -8,25 +8,22 @@ namespace Tangram { class LinuxPlatform : public Platform { - public: - LinuxPlatform(); - LinuxPlatform(UrlClient::Options urlClientOptions); + explicit LinuxPlatform(UrlClient::Options urlClientOptions); ~LinuxPlatform() override; + void shutdown() override; void requestRender() const override; std::vector systemFontFallbacksHandle() const override; FontSourceHandle systemFont(const std::string& _name, const std::string& _weight, - const std::string& _face) const override; + const std::string& _face) const override; UrlRequestHandle startUrlRequest(Url _url, UrlCallback _callback) override; void cancelUrlRequest(UrlRequestHandle _request) override; protected: - - FcConfig* m_fcConfig = nullptr; - UrlClient m_urlClient; - + std::unique_ptr m_urlClient; + bool m_shutdown = false; }; } // namespace Tangram diff --git a/platforms/osx/src/osxPlatform.h b/platforms/osx/src/osxPlatform.h index 3883d746fd..c767bf578f 100644 --- a/platforms/osx/src/osxPlatform.h +++ b/platforms/osx/src/osxPlatform.h @@ -13,6 +13,7 @@ class OSXPlatform : public Platform { OSXPlatform(); ~OSXPlatform() override; + void shutdown() override {} void requestRender() const override; std::vector systemFontFallbacksHandle() const override; UrlRequestHandle startUrlRequest(Url _url, UrlCallback _callback) override; diff --git a/tests/src/mockPlatform.h b/tests/src/mockPlatform.h index fe80aa21c8..d87519db81 100644 --- a/tests/src/mockPlatform.h +++ b/tests/src/mockPlatform.h @@ -10,6 +10,7 @@ class MockPlatform : public Platform { public: + void shutdown() override {} void requestRender() const override; std::vector systemFontFallbacksHandle() const override; UrlRequestHandle startUrlRequest(Url _url, UrlCallback _callback) override; From 00debce330df33914eb323fb77f0ae15ca5ada7b Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Sat, 17 Nov 2018 20:58:35 +0100 Subject: [PATCH 2/6] Linux: don't let http requests block reading local files --- platforms/linux/src/linuxPlatform.cpp | 30 ++++++++++++++++++++------- platforms/linux/src/linuxPlatform.h | 5 ++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/platforms/linux/src/linuxPlatform.cpp b/platforms/linux/src/linuxPlatform.cpp index 3c305bb96c..40e36bbead 100644 --- a/platforms/linux/src/linuxPlatform.cpp +++ b/platforms/linux/src/linuxPlatform.cpp @@ -25,8 +25,8 @@ void logMsg(const char* fmt, ...) { va_end(args); } -LinuxPlatform::LinuxPlatform() : - m_urlClient(std::make_unique(UrlClient::Options{})) { +LinuxPlatform::LinuxPlatform() { + m_urlClient = std::make_unique(UrlClient::Options{}); m_fcConfig = FcInitLoadConfigAndFonts(); } @@ -80,15 +80,31 @@ FontSourceHandle LinuxPlatform::systemFont(const std::string& _name, UrlRequestHandle LinuxPlatform::startUrlRequest(Url _url, UrlCallback _callback) { if (m_shutdown) { return 0; } - return m_urlClient->addRequest(_url.string(), - [this, cb = _callback](UrlResponse&& r) { - cb(std::move(r)); - requestRender(); - }); + if (_url.hasHttpScheme()) { + return m_urlClient->addRequest(_url.string(), + [this, cb = _callback](UrlResponse&& r) { + cb(std::move(r)); + requestRender(); + }); + } else { + m_fileWorker.enqueue([path = _url.path(), _callback](){ + UrlResponse response; + auto allocator = [&](size_t size) { + response.content.resize(size); + return response.content.data(); + }; + + Platform::bytesFromFileSystem(path.c_str(), allocator); + _callback(std::move(response)); + }); + return std::numeric_limits::max(); + } } void LinuxPlatform::cancelUrlRequest(UrlRequestHandle _request) { if (m_shutdown) { return; } + if (_request == std::numeric_limits::max()) { return; } + m_urlClient->cancelRequest(_request); } diff --git a/platforms/linux/src/linuxPlatform.h b/platforms/linux/src/linuxPlatform.h index fc482c031f..56778e4ff5 100644 --- a/platforms/linux/src/linuxPlatform.h +++ b/platforms/linux/src/linuxPlatform.h @@ -1,9 +1,11 @@ #pragma once #include +#include #include "platform.h" #include "urlClient.h" +#include "util/asyncWorker.h" namespace Tangram { @@ -23,7 +25,8 @@ class LinuxPlatform : public Platform { protected: FcConfig* m_fcConfig = nullptr; std::unique_ptr m_urlClient; - bool m_shutdown = false; + AsyncWorker m_fileWorker; + std::atomic m_shutdown{false}; }; } // namespace Tangram From f2dfcf53730aa1d3ba3ebe1bfa4edf5f990c5ddf Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Fri, 16 Nov 2018 00:17:32 +0100 Subject: [PATCH 3/6] Android: load files asynchronously --- .../android/tangram/src/main/cpp/androidPlatform.cpp | 11 ++++++----- .../android/tangram/src/main/cpp/androidPlatform.h | 5 ++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp index 7fa2685d8e..face868572 100644 --- a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp +++ b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp @@ -292,14 +292,15 @@ UrlRequestHandle AndroidPlatform::startUrlRequest(Url _url, UrlCallback _callbac // Get the current value of the request counter and add one, atomically. UrlRequestHandle requestHandle = m_urlRequestCount++; + if (!_callback) { return requestHandle; } // If the requested URL does not use HTTP or HTTPS, retrieve it synchronously. if (!_url.hasHttpScheme()) { - UrlResponse response; - response.content = bytesFromFile(_url); - if (_callback) { - _callback(std::move(response)); - } + m_fileWorker.enqueue([=](){ + UrlResponse response; + response.content = bytesFromFile(_url); + _callback(std::move(response)); + }); return requestHandle; } diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.h b/platforms/android/tangram/src/main/cpp/androidPlatform.h index 5e914081de..b6649165e9 100644 --- a/platforms/android/tangram/src/main/cpp/androidPlatform.h +++ b/platforms/android/tangram/src/main/cpp/androidPlatform.h @@ -1,10 +1,11 @@ #pragma once #include "platform.h" +#include "util/asyncWorker.h" + #include #include - #include #include #include @@ -65,6 +66,8 @@ class AndroidPlatform : public Platform { std::mutex m_callbackMutex; std::unordered_map m_callbacks; + AsyncWorker m_fileWorker; + }; } // namespace Tangram From 0edb840de3bd7865d1961eb1d5f31d1024707a90 Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Thu, 6 Dec 2018 21:27:31 +0100 Subject: [PATCH 4/6] Android JNI: call from less threads - enqueue async calls on single thread --- .../tangram/src/main/cpp/androidPlatform.cpp | 94 ++++++++++--------- .../tangram/src/main/cpp/androidPlatform.h | 2 + .../android/tangram/src/main/cpp/jniWorker.h | 69 ++++++++++++++ 3 files changed, 122 insertions(+), 43 deletions(-) create mode 100644 platforms/android/tangram/src/main/cpp/jniWorker.h diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp index face868572..06ef562767 100644 --- a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp +++ b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp @@ -132,10 +132,14 @@ class JniThreadBinding { public: JniThreadBinding(JavaVM* _jvm) : jvm(_jvm) { status = jvm->GetEnv((void**)&jniEnv, TANGRAM_JNI_VERSION); - if (status == JNI_EDETACHED) { jvm->AttachCurrentThread(&jniEnv, NULL);} + if (status == JNI_EDETACHED) { + jvm->AttachCurrentThread(&jniEnv, NULL); + } } ~JniThreadBinding() { - if (status == JNI_EDETACHED) { jvm->DetachCurrentThread(); } + if (status == JNI_EDETACHED) { + jvm->DetachCurrentThread(); + } } JNIEnv* operator->() const { @@ -172,7 +176,9 @@ std::string AndroidPlatform::fontPath(const std::string& _family, const std::str return resultStr; } -AndroidPlatform::AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance) { +AndroidPlatform::AndroidPlatform(JNIEnv* _jniEnv, jobject _assetManager, jobject _tangramInstance) + : m_jniWorker(jvm) { + m_tangramInstance = _jniEnv->NewGlobalRef(_tangramInstance); m_assetManager = AAssetManager_fromJava(_jniEnv, _assetManager); @@ -190,30 +196,31 @@ void AndroidPlatform::dispose(JNIEnv* _jniEnv) { } void AndroidPlatform::requestRender() const { - - JniThreadBinding jniEnv(jvm); - - jniEnv->CallVoidMethod(m_tangramInstance, requestRenderMethodID); + m_jniWorker.enqueue([&](JNIEnv *jniEnv) { + jniEnv->CallVoidMethod(m_tangramInstance, requestRenderMethodID); + }); } -std::string AndroidPlatform::fontFallbackPath(int _importance, int _weightHint) const { - +std::vector AndroidPlatform::systemFontFallbacksHandle() const { JniThreadBinding jniEnv(jvm); - jstring returnStr = (jstring) jniEnv->CallObjectMethod(m_tangramInstance, getFontFallbackFilePath, _importance, _weightHint); - - auto resultStr = stringFromJString(jniEnv, returnStr); - jniEnv->DeleteLocalRef(returnStr); - - return resultStr; -} - -std::vector AndroidPlatform::systemFontFallbacksHandle() const { std::vector handles; int importance = 0; int weightHint = 400; + auto fontFallbackPath = [&](int _importance, int _weightHint) { + + jstring returnStr = (jstring) jniEnv->CallObjectMethod(m_tangramInstance, + getFontFallbackFilePath, _importance, + _weightHint); + + auto resultStr = stringFromJString(jniEnv, returnStr); + jniEnv->DeleteLocalRef(returnStr); + + return resultStr; + }; + std::string fallbackPath = fontFallbackPath(importance, weightHint); while (!fallbackPath.empty()) { @@ -288,8 +295,6 @@ std::vector AndroidPlatform::bytesFromFile(const Url& url) const { UrlRequestHandle AndroidPlatform::startUrlRequest(Url _url, UrlCallback _callback) { - JniThreadBinding jniEnv(jvm); - // Get the current value of the request counter and add one, atomically. UrlRequestHandle requestHandle = m_urlRequestCount++; if (!_callback) { return requestHandle; } @@ -310,26 +315,28 @@ UrlRequestHandle AndroidPlatform::startUrlRequest(Url _url, UrlCallback _callbac m_callbacks[requestHandle] = _callback; } - jlong jRequestHandle = static_cast(requestHandle); + m_jniWorker.enqueue([=](JNIEnv *jniEnv) { + jlong jRequestHandle = static_cast(requestHandle); - // Check that it's safe to convert the UrlRequestHandle to a jlong and back. - assert(requestHandle == static_cast(jRequestHandle)); + // Check that it's safe to convert the UrlRequestHandle to a jlong and back... cmon :P + assert(requestHandle == static_cast(jRequestHandle)); - jstring jUrl = jstringFromString(jniEnv, _url.string()); - - // Call the MapController method to start the URL request. - jniEnv->CallVoidMethod(m_tangramInstance, startUrlRequestMID, jUrl, jRequestHandle); + jstring jUrl = jstringFromString(jniEnv, _url.string()); + // Call the MapController method to start the URL request. + jniEnv->CallVoidMethod(m_tangramInstance, startUrlRequestMID, jUrl, jRequestHandle); + }); return requestHandle; } void AndroidPlatform::cancelUrlRequest(UrlRequestHandle request) { - JniThreadBinding jniEnv(jvm); + m_jniWorker.enqueue([=](JNIEnv *jniEnv) { - jlong jRequestHandle = static_cast(request); + jlong jRequestHandle = static_cast(request); - jniEnv->CallVoidMethod(m_tangramInstance, cancelUrlRequestMID, jRequestHandle); + jniEnv->CallVoidMethod(m_tangramInstance, cancelUrlRequestMID, jRequestHandle); + }); // We currently don't try to cancel requests for local files. } @@ -353,20 +360,21 @@ void AndroidPlatform::onUrlComplete(JNIEnv* _jniEnv, jlong _jRequestHandle, jbyt response.error = error.c_str(); } - // Find the callback associated with the request. - UrlCallback callback; - { - std::lock_guard lock(m_callbackMutex); - UrlRequestHandle requestHandle = static_cast(_jRequestHandle); - auto it = m_callbacks.find(requestHandle); - if (it != m_callbacks.end()) { - callback = std::move(it->second); - m_callbacks.erase(it); + + m_fileWorker.enqueue([this, _jRequestHandle, r = std::move(response)]() mutable { + // Find the callback associated with the request. + UrlCallback callback; + { + std::lock_guard lock(m_callbackMutex); + UrlRequestHandle requestHandle = static_cast(_jRequestHandle); + auto it = m_callbacks.find(requestHandle); + if (it != m_callbacks.end()) { + callback = std::move(it->second); + m_callbacks.erase(it); + } } - } - if (callback) { - callback(std::move(response)); - } + if (callback) { callback(std::move(r)); } + }); } void setCurrentThreadPriority(int priority) { diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.h b/platforms/android/tangram/src/main/cpp/androidPlatform.h index b6649165e9..38fbb36147 100644 --- a/platforms/android/tangram/src/main/cpp/androidPlatform.h +++ b/platforms/android/tangram/src/main/cpp/androidPlatform.h @@ -1,6 +1,7 @@ #pragma once #include "platform.h" +#include "jniWorker.h" #include "util/asyncWorker.h" @@ -66,6 +67,7 @@ class AndroidPlatform : public Platform { std::mutex m_callbackMutex; std::unordered_map m_callbacks; + mutable JniWorker m_jniWorker; // FIX requestRender const.. Lets use Rust if we want this for real AsyncWorker m_fileWorker; }; diff --git a/platforms/android/tangram/src/main/cpp/jniWorker.h b/platforms/android/tangram/src/main/cpp/jniWorker.h new file mode 100644 index 0000000000..0586ac9f90 --- /dev/null +++ b/platforms/android/tangram/src/main/cpp/jniWorker.h @@ -0,0 +1,69 @@ +#include +#include +#include +#include +#include + +namespace Tangram { + +class JniWorker { +public: + + explicit JniWorker(JavaVM* _jvm) : jvm(_jvm){ + + thread = std::thread(&JniWorker::run, this); + + pthread_setname_np(thread.native_handle(), "TangramJNI Worker"); + } + + ~JniWorker() { + { + std::unique_lock lock(m_mutex); + m_running = false; + } + m_condition.notify_all(); + thread.join(); + } + + void enqueue(std::function _task) { + { + std::unique_lock lock(m_mutex); + if (!m_running) { return; } + + m_queue.push_back(std::move(_task)); + } + m_condition.notify_one(); + } + +private: + + void run() { + jvm->AttachCurrentThread(&jniEnv, NULL); + + while (true) { + std::function task; + { + std::unique_lock lock(m_mutex); + m_condition.wait(lock, [&]{ return !m_running || !m_queue.empty(); }); + if (!m_running) { break; } + + task = std::move(m_queue.front()); + m_queue.pop_front(); + } + task(jniEnv); + } + jvm->DetachCurrentThread(); + } + + std::thread thread; + bool m_running = true; + std::condition_variable m_condition; + std::mutex m_mutex; + std::deque> m_queue; + + JavaVM* jvm; + JNIEnv *jniEnv; + +}; + +} From 32499ba6ae2aa5741d822cb780ce1bfb12997235 Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Thu, 6 Dec 2018 21:27:47 +0100 Subject: [PATCH 5/6] drop logging --- platforms/android/tangram/src/main/cpp/androidPlatform.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp index 06ef562767..e2e927bbe0 100644 --- a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp +++ b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp @@ -133,11 +133,13 @@ class JniThreadBinding { JniThreadBinding(JavaVM* _jvm) : jvm(_jvm) { status = jvm->GetEnv((void**)&jniEnv, TANGRAM_JNI_VERSION); if (status == JNI_EDETACHED) { + LOG("---------------->>> ATTACH"); jvm->AttachCurrentThread(&jniEnv, NULL); } } ~JniThreadBinding() { if (status == JNI_EDETACHED) { + LOG("---------------->>> DETACH"); jvm->DetachCurrentThread(); } } From 908eeefe966eeff90c36157bc8bf45008b87db49 Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Mon, 19 Nov 2018 13:28:29 +0100 Subject: [PATCH 6/6] Use WeakGlobalRef for android MapController instance - MapController is calling dispose, so it must be alive here --- .../android/tangram/src/main/cpp/androidPlatform.cpp | 6 +----- .../android/tangram/src/main/cpp/androidPlatform.h | 1 - platforms/android/tangram/src/main/cpp/jniExports.cpp | 11 ++--------- .../main/java/com/mapzen/tangram/MapController.java | 4 ++-- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/platforms/android/tangram/src/main/cpp/androidPlatform.cpp b/platforms/android/tangram/src/main/cpp/androidPlatform.cpp index e2e927bbe0..701252555e 100644 --- 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 index 38fbb36147..9bbe615a5f 100644 --- 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 index 2fb38b7e0a..1732a2cf26 100644 --- 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(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(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(*platform).dispose(jniEnv); + delete reinterpret_cast(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 index f73e1a9a23..12c67bd972 100644 --- 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);