From 19e7d154bb540302339eb984f5350c20e9001020 Mon Sep 17 00:00:00 2001 From: Leith Bade Date: Thu, 17 Sep 2015 17:34:13 -0700 Subject: [PATCH] Improve handling of cancelled HTTP requests Fix a memory leak of NativeMapView Java object --- android/cpp/native_map_view.cpp | 4 +-- .../com/mapbox/mapboxgl/http/HTTPContext.java | 4 +++ include/mbgl/android/native_map_view.hpp | 2 +- platform/android/http_request_android.cpp | 35 +++++++++++-------- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/android/cpp/native_map_view.cpp b/android/cpp/native_map_view.cpp index 02228d85b..6c1cd87bb 100644 --- a/android/cpp/native_map_view.cpp +++ b/android/cpp/native_map_view.cpp @@ -73,7 +73,7 @@ NativeMapView::NativeMapView(JNIEnv *env, jobject obj_, float pixelRatio_, int a return; } - obj = env->NewGlobalRef(obj_); + obj = env->NewWeakGlobalRef(obj_); if (obj == nullptr) { env->ExceptionDescribe(); return; @@ -93,7 +93,7 @@ NativeMapView::~NativeMapView() { JNIEnv *env = nullptr; ret = vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6); if (ret == JNI_OK) { - env->DeleteGlobalRef(obj); + env->DeleteWeakGlobalRef(obj); } else { mbgl::Log::Error(mbgl::Event::JNI, "GetEnv() failed with %i", ret); throw new std::runtime_error("GetEnv() failed"); diff --git a/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/http/HTTPContext.java b/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/http/HTTPContext.java index 4558bbfe3..775daaafd 100644 --- a/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/http/HTTPContext.java +++ b/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/http/HTTPContext.java @@ -20,6 +20,7 @@ class HTTPContext { private static final int CONNECTION_ERROR = 0; private static final int TEMPORARY_ERROR = 1; private static final int PERMANENT_ERROR = 2; + private static final int CANCELED_ERROR = 3; private static HTTPContext mInstance = null; @@ -78,7 +79,10 @@ class HTTPContext { type = CONNECTION_ERROR; } else if ((e instanceof InterruptedIOException)) { type = TEMPORARY_ERROR; + } else if (mCall.isCanceled()) { + type = CANCELED_ERROR; } + nativeOnFailure(mNativePtr, type, e.getMessage()); } diff --git a/include/mbgl/android/native_map_view.hpp b/include/mbgl/android/native_map_view.hpp index 4033c628b..0a4e856db 100644 --- a/include/mbgl/android/native_map_view.hpp +++ b/include/mbgl/android/native_map_view.hpp @@ -62,7 +62,7 @@ private: private: JavaVM *vm = nullptr; - jobject obj = nullptr; + jweak obj = nullptr; ANativeWindow *window = nullptr; EGLDisplay display = EGL_NO_DISPLAY; diff --git a/platform/android/http_request_android.cpp b/platform/android/http_request_android.cpp index 249cb731e..5c5805906 100644 --- a/platform/android/http_request_android.cpp +++ b/platform/android/http_request_android.cpp @@ -75,7 +75,8 @@ private: static const int connectionError = 0; static const int temporaryError = 1; - static const int permanentError = 1; + static const int permanentError = 2; + static const int canceledError = 3; }; // ------------------------------------------------------------------------------------------------- @@ -305,22 +306,26 @@ void HTTPAndroidRequest::onResponse(int code, std::string message, std::string e } void HTTPAndroidRequest::onFailure(int type, std::string message) { - if (!response) { - response = std::make_unique(); - } + if (type != canceledError) { + if (!response) { + response = std::make_unique(); + } - response->status = Response::Error; - response->message = message; + response->status = Response::Error; + response->message = message; - switch (type) { - case connectionError: - status = ResponseStatus::ConnectionError; - - case temporaryError: - status = ResponseStatus::TemporaryError; - - default: - status = ResponseStatus::PermanentError; + switch (type) { + case connectionError: + status = ResponseStatus::ConnectionError; + break; + case temporaryError: + status = ResponseStatus::TemporaryError; + break; + default: + status = ResponseStatus::PermanentError; + } + } else { + status = ResponseStatus::Canceled; } async.send();