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

Deleting the global ref of the observerCallback in JNI makes the offline activity to crash #4121

Merged
merged 1 commit into from
Mar 5, 2016
Merged

Deleting the global ref of the observerCallback in JNI makes the offline activity to crash #4121

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 30 additions & 13 deletions platform/android/src/jni.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,12 +1469,22 @@ void setOfflineMapboxTileCountLimit(JNIEnv *env, jni::jobject* obj, jlong defaul
defaultFileSource->setOfflineMapboxTileCountLimit(limit);
}

void destroyOfflineRegion(JNIEnv *env, jni::jobject* obj, jlong offlineRegionPtr) {
void destroyOfflineRegion(JNIEnv *env, jni::jobject* offlineRegion_, jlong) {
mbgl::Log::Debug(mbgl::Event::JNI, "destroyOfflineRegion");
assert(offlineRegionPtr != 0);

// Offline region
jlong offlineRegionPtr = jni::GetField<jlong>(*env, offlineRegion_, *offlineRegionPtrId);
mbgl::OfflineRegion *offlineRegion = reinterpret_cast<mbgl::OfflineRegion *>(offlineRegionPtr);

// File source
jni::jobject* jmanager = jni::GetField<jni::jobject*>(*env, offlineRegion_, *offlineRegionOfflineManagerId);
jlong defaultFileSourcePtr = jni::GetField<jlong>(*env, jmanager, *offlineManagerClassPtrId);
mbgl::DefaultFileSource *defaultFileSource = reinterpret_cast<mbgl::DefaultFileSource *>(defaultFileSourcePtr);

// Release the observer
defaultFileSource->setOfflineRegionObserver(*offlineRegion, nullptr);

delete offlineRegion;
offlineRegion = nullptr;
}

void setOfflineRegionObserver(JNIEnv *env, jni::jobject* obj, jni::jobject* offlineRegion_, jni::jobject* observerCallback) {
Expand All @@ -1492,8 +1502,17 @@ void setOfflineRegionObserver(JNIEnv *env, jni::jobject* obj, jni::jobject* offl
// Define the observer
class Observer : public mbgl::OfflineRegionObserver {
public:
Observer(jni::jobject* observerCallback_)
: observerCallback(observerCallback_) {
Observer(jni::UniqueGlobalRef<jni::jobject>&& observerCallback_)
: observerCallback(std::move(observerCallback_)) {
}

~Observer() override {
mbgl::Log::Debug(mbgl::Event::JNI, "~Observer()");
// Env
JNIEnv* env2;
jboolean renderDetach = attach_jni_thread(theJVM, &env2, "Offline Thread");
jni::DeleteGlobalRef(*env2, std::move(observerCallback));
detach_jni_thread(theJVM, &env2, renderDetach);
}

void statusChanged(mbgl::OfflineRegionStatus status) override {
Expand All @@ -1519,7 +1538,7 @@ void setOfflineRegionObserver(JNIEnv *env, jni::jobject* obj, jni::jobject* offl
jni::SetField<jlong>(*env2, jstatus, *offlineRegionStatusCompletedResourceSizeId, status.completedResourceSize);
jni::SetField<jlong>(*env2, jstatus, *offlineRegionStatusRequiredResourceCountId, status.requiredResourceCount);
jni::SetField<jboolean>(*env2, jstatus, *offlineRegionStatusRequiredResourceCountIsPreciseId, status.requiredResourceCountIsPrecise);
jni::CallMethod<void>(*env2, observerCallback, *offlineRegionObserveronStatusChangedId, jstatus);
jni::CallMethod<void>(*env2, observerCallback.get(), *offlineRegionObserveronStatusChangedId, jstatus);

// Detach when we're done
detach_jni_thread(theJVM, &env2, renderDetach);
Expand Down Expand Up @@ -1554,7 +1573,7 @@ void setOfflineRegionObserver(JNIEnv *env, jni::jobject* obj, jni::jobject* offl
jni::jobject* jerror = &jni::NewObject(*env2, *offlineRegionErrorClass, *offlineRegionErrorConstructorId);
jni::SetField<jni::jobject*>(*env2, jerror, *offlineRegionErrorReasonId, std_string_to_jstring(env2, errorReason));
jni::SetField<jni::jobject*>(*env2, jerror, *offlineRegionErrorMessageId, std_string_to_jstring(env2, error.message));
jni::CallMethod<void>(*env2, observerCallback, *offlineRegionObserveronErrorId, jerror);
jni::CallMethod<void>(*env2, observerCallback.get(), *offlineRegionObserveronErrorId, jerror);

// Detach when we're done
detach_jni_thread(theJVM, &env2, renderDetach);
Expand All @@ -1566,20 +1585,18 @@ void setOfflineRegionObserver(JNIEnv *env, jni::jobject* obj, jni::jobject* offl
jboolean renderDetach = attach_jni_thread(theJVM, &env2, "Offline Thread");

// Send limit
jni::CallMethod<void>(*env2, observerCallback, *offlineRegionObserveronLimitId, jlong(limit));
jni::CallMethod<void>(*env2, observerCallback.get(), *offlineRegionObserveronLimitId, jlong(limit));

// Detach when we're done
detach_jni_thread(theJVM, &env2, renderDetach);
}

jni::jobject* observerCallback;
jni::UniqueGlobalRef<jni::jobject> observerCallback;
};

// Makes sure the callback doesn't get GC'ed
observerCallback = jni::NewGlobalRef(*env, observerCallback).release();

// Set the observer
defaultFileSource->setOfflineRegionObserver(*offlineRegion, std::make_unique<Observer>(observerCallback));
defaultFileSource->setOfflineRegionObserver(*offlineRegion,
std::make_unique<Observer>(jni::NewGlobalRef(*env, observerCallback)));
}

void setOfflineRegionDownloadState(JNIEnv *env, jni::jobject* obj, jni::jobject* offlineRegion_, jint offlineRegionDownloadState) {
Expand Down