From 3111e5fb48dd5c58e5669119081add277c1dd068 Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Tue, 7 Nov 2017 12:33:43 +0100 Subject: [PATCH] Android: Replacement for JNIEnv::FindClass that works from any thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL adds a replacement for JNIEnv::FindClass that works from any thread, i.e. from native C++ threads as well. This function will be used from the generated JNI code. Long term, we should stop using classreferenceholder that relies on a hardcoded list of WebRTC classes. Bug: webrtc:8278 Change-Id: I4f40c744325ac02b73bd8fa479ab50b684429dc2 Reviewed-on: https://webrtc-review.googlesource.com/20223 Commit-Queue: Magnus Jedvert Reviewed-by: Sami Kalliomäki Cr-Commit-Position: refs/heads/master@{#20583} --- sdk/android/BUILD.gn | 12 +++ .../src/java/org/webrtc/ClassLoader.java | 23 ++++++ sdk/android/src/jni/class_loader.cc | 80 +++++++++++++++++++ sdk/android/src/jni/class_loader.h | 40 ++++++++++ sdk/android/src/jni/classreferenceholder.cc | 1 - sdk/android/src/jni/classreferenceholder.h | 6 ++ sdk/android/src/jni/jni_generator_helper.cc | 18 +---- sdk/android/src/jni/jni_onload.cc | 3 + 8 files changed, 168 insertions(+), 15 deletions(-) create mode 100644 sdk/android/src/java/org/webrtc/ClassLoader.java create mode 100644 sdk/android/src/jni/class_loader.cc create mode 100644 sdk/android/src/jni/class_loader.h diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index f30c7aef9d..b2bef9ed3c 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -33,9 +33,19 @@ config("libjingle_peerconnection_jni_warnings_config") { } } +generate_jni("generated_base_jni") { + sources = [ + "src/java/org/webrtc/ClassLoader.java", + ] + jni_package = "" + jni_generator_include = "//sdk/android/src/jni/jni_generator_helper.h" +} + rtc_source_set("base_jni") { sources = [ "src/jni/androidhistogram_jni.cc", + "src/jni/class_loader.cc", + "src/jni/class_loader.h", "src/jni/classreferenceholder.cc", "src/jni/classreferenceholder.h", "src/jni/jni_common.cc", @@ -49,6 +59,7 @@ rtc_source_set("base_jni") { ] deps = [ + ":generated_base_jni", "../../api:libjingle_peerconnection_api", "../../rtc_base:rtc_base", "../../rtc_base:rtc_base_approved", @@ -465,6 +476,7 @@ android_library("libjingle_peerconnection_java") { "src/java/org/webrtc/Camera2Session.java", "src/java/org/webrtc/CameraCapturer.java", "src/java/org/webrtc/CameraSession.java", + "src/java/org/webrtc/ClassLoader.java", "src/java/org/webrtc/DynamicBitrateAdjuster.java", "src/java/org/webrtc/EglBase10.java", "src/java/org/webrtc/EglBase14.java", diff --git a/sdk/android/src/java/org/webrtc/ClassLoader.java b/sdk/android/src/java/org/webrtc/ClassLoader.java new file mode 100644 index 0000000000..463704607c --- /dev/null +++ b/sdk/android/src/java/org/webrtc/ClassLoader.java @@ -0,0 +1,23 @@ +/* + * Copyright 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +package org.webrtc; + +/** + * This class provides a ClassLoader that is capable of loading WebRTC Java classes regardless of + * what thread it's called from. Such a ClassLoader is needed for the few cases where the JNI + * mechanism is unable to automatically determine the appropriate ClassLoader instance. + */ +class ClassLoader { + @CalledByNative + static Object getClassLoader() { + return ClassLoader.class.getClassLoader(); + } +} diff --git a/sdk/android/src/jni/class_loader.cc b/sdk/android/src/jni/class_loader.cc new file mode 100644 index 0000000000..c21e6c0e8d --- /dev/null +++ b/sdk/android/src/jni/class_loader.cc @@ -0,0 +1,80 @@ +/* + * Copyright 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "sdk/android/src/jni/class_loader.h" + +#include +#include + +#include "rtc_base/checks.h" +#include "sdk/android/generated_base_jni/jni/ClassLoader_jni.h" + +// Abort the process if |jni| has a Java exception pending. This macros uses the +// comma operator to execute ExceptionDescribe and ExceptionClear ignoring their +// return values and sending "" to the error stream. +#define CHECK_EXCEPTION(jni) \ + RTC_CHECK(!jni->ExceptionCheck()) \ + << (jni->ExceptionDescribe(), jni->ExceptionClear(), "") + +namespace webrtc { +namespace jni { + +namespace { + +class ClassLoader { + public: + explicit ClassLoader(JNIEnv* env) { + class_loader_class_ = reinterpret_cast( + env->NewGlobalRef(env->FindClass("java/lang/ClassLoader"))); + CHECK_EXCEPTION(env); + load_class_method_ = + env->GetMethodID(class_loader_class_, "loadClass", + "(Ljava/lang/String;)Ljava/lang/Class;"); + CHECK_EXCEPTION(env); + class_loader_ = env->NewGlobalRef(Java_ClassLoader_getClassLoader(env)); + CHECK_EXCEPTION(env); + } + + jclass FindClass(JNIEnv* env, const char* c_name) { + // ClassLoader.loadClass expects a classname with components separated by + // dots instead of the slashes that JNIEnv::FindClass expects. + std::string name(c_name); + std::replace(name.begin(), name.end(), '/', '.'); + jstring jstr = env->NewStringUTF(name.c_str()); + const jclass clazz = static_cast( + env->CallObjectMethod(class_loader_, load_class_method_, jstr)); + CHECK_EXCEPTION(env); + return clazz; + } + + private: + jclass class_loader_class_; + jmethodID load_class_method_; + jobject class_loader_; +}; + +static ClassLoader* g_class_loader = nullptr; + +} // namespace + +void InitClassLoader(JNIEnv* env) { + RTC_CHECK(g_class_loader == nullptr); + g_class_loader = new ClassLoader(env); +} + +jclass GetClass(JNIEnv* env, const char* name) { + // The class loader will be null in the JNI code called from the ClassLoader + // ctor when we are bootstrapping ourself. + return (g_class_loader == nullptr) ? env->FindClass(name) + : g_class_loader->FindClass(env, name); +} + +} // namespace jni +} // namespace webrtc diff --git a/sdk/android/src/jni/class_loader.h b/sdk/android/src/jni/class_loader.h new file mode 100644 index 0000000000..52bb1ba523 --- /dev/null +++ b/sdk/android/src/jni/class_loader.h @@ -0,0 +1,40 @@ +/* + * Copyright 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +// Android's FindClass() is tricky because the app-specific ClassLoader is not +// consulted when there is no app-specific frame on the stack (i.e. when called +// from a thread created from native C++ code). These helper functions provide a +// workaround for this. +// http://developer.android.com/training/articles/perf-jni.html#faq_FindClass + +#ifndef SDK_ANDROID_SRC_JNI_CLASS_LOADER_H_ +#define SDK_ANDROID_SRC_JNI_CLASS_LOADER_H_ + +#include + +namespace webrtc { +namespace jni { + +// This method should be called from JNI_OnLoad and before any calls to +// FindClass. +void InitClassLoader(JNIEnv* env); + +// This function is identical to JNIEnv::FindClass except that it works from any +// thread. This function loads and returns a local reference to the class with +// the given name. The name argument is a fully-qualified class name. For +// example, the fully-qualified class name for the java.lang.String class is: +// "java/lang/String". This function will be used from the JNI generated code +// and should rarely be used manually. +jclass GetClass(JNIEnv* env, const char* name); + +} // namespace jni +} // namespace webrtc + +#endif // SDK_ANDROID_SRC_JNI_CLASS_LOADER_H_ diff --git a/sdk/android/src/jni/classreferenceholder.cc b/sdk/android/src/jni/classreferenceholder.cc index 3943cf227f..e4ff16f88d 100644 --- a/sdk/android/src/jni/classreferenceholder.cc +++ b/sdk/android/src/jni/classreferenceholder.cc @@ -111,7 +111,6 @@ ClassReferenceHolder::ClassReferenceHolder(JNIEnv* jni) { LoadClass(jni, "org/webrtc/VideoCodecStatus"); LoadClass(jni, "org/webrtc/VideoDecoder$Settings"); LoadClass(jni, "org/webrtc/VideoDecoderWrapperCallback"); - LoadClass(jni, "org/webrtc/VideoEncoder"); LoadClass(jni, "org/webrtc/VideoEncoder$BitrateAllocation"); LoadClass(jni, "org/webrtc/VideoEncoder$EncodeInfo"); LoadClass(jni, "org/webrtc/VideoEncoder$ScalingSettings"); diff --git a/sdk/android/src/jni/classreferenceholder.h b/sdk/android/src/jni/classreferenceholder.h index 7d8658b611..5145c108e6 100644 --- a/sdk/android/src/jni/classreferenceholder.h +++ b/sdk/android/src/jni/classreferenceholder.h @@ -16,6 +16,9 @@ #ifndef SDK_ANDROID_SRC_JNI_CLASSREFERENCEHOLDER_H_ #define SDK_ANDROID_SRC_JNI_CLASSREFERENCEHOLDER_H_ +// TODO(magjed): Remove this whole file and replace with either generated JNI +// code or class_loader.h. + #include #include #include @@ -28,6 +31,9 @@ void LoadGlobalClassReferenceHolder(); // FreeGlobalClassReferenceHolder must be called in JNI_UnLoad. void FreeGlobalClassReferenceHolder(); +// Deprecated. Most cases of finding classes should be done with generated JNI +// code, and the few remaining cases should use the function from +// class_loader.h. // Returns a global reference guaranteed to be valid for the lifetime of the // process. jclass FindClass(JNIEnv* jni, const char* name); diff --git a/sdk/android/src/jni/jni_generator_helper.cc b/sdk/android/src/jni/jni_generator_helper.cc index 5a0952374d..c870e1c51c 100644 --- a/sdk/android/src/jni/jni_generator_helper.cc +++ b/sdk/android/src/jni/jni_generator_helper.cc @@ -11,22 +11,11 @@ #include "sdk/android/src/jni/jni_generator_helper.h" #include "rtc_base/atomicops.h" -#include "sdk/android/src/jni/classreferenceholder.h" +#include "sdk/android/src/jni/class_loader.h" namespace base { namespace android { -namespace { -// JNIEnv-helper methods that RTC_CHECK success: no Java exception thrown and -// found object/class/method/field is non-null. -jclass GetClass(JNIEnv* jni, const char* class_name) { - jclass clazz = webrtc::jni::FindClass(jni, class_name); - CHECK_EXCEPTION(jni) << "error during FindClass: " << class_name; - RTC_CHECK(clazz) << class_name; - return clazz; -} -} // namespace - // If |atomic_class_id| set, it'll return immediately. Otherwise, it will look // up the class and store it. If there's a race, we take care to only store one // global reference (and the duplicated effort will happen only once). @@ -39,8 +28,9 @@ jclass LazyGetClass(JNIEnv* env, rtc::AtomicOps::AcquireLoadPtr(atomic_class_id); if (value) return reinterpret_cast(value); - jclass clazz = - static_cast(env->NewGlobalRef(GetClass(env, class_name))); + jclass clazz = static_cast( + env->NewGlobalRef(webrtc::jni::GetClass(env, class_name))); + RTC_CHECK(clazz) << class_name; base::subtle::AtomicWord null_aw = nullptr; base::subtle::AtomicWord cas_result = rtc::AtomicOps::CompareAndSwapPtr( atomic_class_id, null_aw, diff --git a/sdk/android/src/jni/jni_onload.cc b/sdk/android/src/jni/jni_onload.cc index 81cc4889c2..cb708e7fda 100644 --- a/sdk/android/src/jni/jni_onload.cc +++ b/sdk/android/src/jni/jni_onload.cc @@ -13,6 +13,7 @@ #define JNIEXPORT __attribute__((visibility("default"))) #include "rtc_base/ssladapter.h" +#include "sdk/android/src/jni/class_loader.h" #include "sdk/android/src/jni/classreferenceholder.h" #include "sdk/android/src/jni/jni_helpers.h" @@ -27,6 +28,8 @@ extern "C" jint JNIEXPORT JNICALL JNI_OnLoad(JavaVM *jvm, void *reserved) { RTC_CHECK(rtc::InitializeSSL()) << "Failed to InitializeSSL()"; LoadGlobalClassReferenceHolder(); + JNIEnv* env = AttachCurrentThreadIfNeeded(); + InitClassLoader(env); return ret; }