diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java index 66bdc4ae7..7be1c1a0f 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java @@ -10,6 +10,7 @@ import java.io.InterruptedIOException; import java.net.ProtocolException; import java.net.SocketException; import java.net.UnknownHostException; +import java.util.concurrent.locks.ReentrantLock; import javax.net.ssl.SSLException; @@ -27,7 +28,10 @@ class HTTPRequest implements Callback { 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; + + // Reentrancy is not needed, but "Lock" is an + // abstract class. + private ReentrantLock mLock = new ReentrantLock(); private long mNativePtr = 0; @@ -52,6 +56,15 @@ class HTTPRequest implements Callback { public void cancel() { mCall.cancel(); + + // TODO: We need a lock here because we can try + // to cancel at the same time the request is getting + // answered on the OkHTTP thread. We could get rid of + // this lock by using Runnable when we move Android + // implementation of mbgl::RunLoop to Looper. + mLock.lock(); + mNativePtr = 0; + mLock.unlock(); } @Override @@ -77,7 +90,11 @@ class HTTPRequest implements Callback { response.body().close(); } - nativeOnResponse(response.code(), response.header("ETag"), response.header("Last-Modified"), response.header("Cache-Control"), response.header("Expires"), body); + mLock.lock(); + if (mNativePtr != 0) { + nativeOnResponse(response.code(), response.header("ETag"), response.header("Last-Modified"), response.header("Cache-Control"), response.header("Expires"), body); + } + mLock.unlock(); } @Override @@ -89,11 +106,14 @@ class HTTPRequest implements Callback { type = CONNECTION_ERROR; } else if ((e instanceof InterruptedIOException)) { type = TEMPORARY_ERROR; - } else if (mCall.isCanceled()) { - type = CANCELED_ERROR; } String errorMessage = e.getMessage() != null ? e.getMessage() : "Error processing the request"; - nativeOnFailure(type, errorMessage); + + mLock.lock(); + if (mNativePtr != 0) { + nativeOnFailure(type, errorMessage); + } + mLock.unlock(); } } diff --git a/platform/android/src/http_request_android.cpp b/platform/android/src/http_request_android.cpp index 3f9556710..1e0039cdf 100644 --- a/platform/android/src/http_request_android.cpp +++ b/platform/android/src/http_request_android.cpp @@ -40,8 +40,6 @@ public: private: void finish(); - bool cancelled = false; - std::unique_ptr response; const std::shared_ptr existingResponse; @@ -50,7 +48,6 @@ private: static const int connectionError = 0; static const int temporaryError = 1; static const int permanentError = 2; - static const int canceledError = 3; }; jni::Class HTTPRequest::javaClass; @@ -97,20 +94,18 @@ HTTPRequest::HTTPRequest(jni::JNIEnv& env, const Resource& resource_, Callback c } void HTTPRequest::cancel() { - cancelled = true; - UniqueEnv env = android::AttachEnv(); static auto cancel = javaClass.GetMethod(*env, "cancel"); javaRequest->Call(*env, cancel); + + delete this; } void HTTPRequest::finish() { - if (!cancelled) { - assert(response); - notify(*response); - } + assert(response); + notify(*response); delete this; } @@ -173,9 +168,6 @@ void HTTPRequest::onFailure(jni::JNIEnv& env, int type, jni::String message) { case temporaryError: response->error = std::make_unique(Error::Reason::Server, messageStr); break; - case canceledError: - response.reset(); - break; default: response->error = std::make_unique(Error::Reason::Other, messageStr); }