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

Commit

Permalink
[android] Do not reach native if the HTTPContext was invalidated
Browse files Browse the repository at this point in the history
Native will get destroyed before the Java HTTPRequest and asynchronous
requests might still get answered.

If the request gets canceled, we now clear the reference to the native
code and it will never reach it.

Lock is needed because OkHTTP replies on its own thread, so we need to
guard the access to the variable we use for checking if the Request was
canceled. In the future we could remove the lock and use Runnable when
we move Android main loop to Looper instead of libuv.

Like the other implementations of HTTPRequest on Mapbox GL Native, the
request is destroyed synchronously on ::cancel().
  • Loading branch information
tmpsantos committed Mar 7, 2016
1 parent 12fa567 commit fe811bd
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.net.ProtocolException;
import java.net.SocketException;
import java.net.UnknownHostException;
import java.util.concurrent.locks.ReentrantLock;

import javax.net.ssl.SSLException;

Expand All @@ -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;

Expand All @@ -52,6 +56,15 @@ private HTTPRequest(long nativePtr, String resourceUrl, String userAgent, String

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
Expand All @@ -77,7 +90,11 @@ public void onResponse(Call call, Response response) throws IOException {
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
Expand All @@ -89,11 +106,14 @@ public void onFailure(Call call, IOException e) {
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();
}
}
16 changes: 4 additions & 12 deletions platform/android/src/http_request_android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ class HTTPRequest : public HTTPRequestBase {
private:
void finish();

bool cancelled = false;

std::unique_ptr<Response> response;
const std::shared_ptr<const Response> existingResponse;

Expand All @@ -50,7 +48,6 @@ class HTTPRequest : public HTTPRequestBase {
static const int connectionError = 0;
static const int temporaryError = 1;
static const int permanentError = 2;
static const int canceledError = 3;
};

jni::Class<HTTPRequest> HTTPRequest::javaClass;
Expand Down Expand Up @@ -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<void ()>(*env, "cancel");

javaRequest->Call(*env, cancel);

delete this;
}

void HTTPRequest::finish() {
if (!cancelled) {
assert(response);
notify(*response);
}
assert(response);
notify(*response);

delete this;
}
Expand Down Expand Up @@ -173,9 +168,6 @@ void HTTPRequest::onFailure(jni::JNIEnv& env, int type, jni::String message) {
case temporaryError:
response->error = std::make_unique<Error>(Error::Reason::Server, messageStr);
break;
case canceledError:
response.reset();
break;
default:
response->error = std::make_unique<Error>(Error::Reason::Other, messageStr);
}
Expand Down

0 comments on commit fe811bd

Please sign in to comment.