From 5e8ba5f05a047259d79a2f2487dc6053a21ae2ec Mon Sep 17 00:00:00 2001 From: Daniel Colascione Date: Thu, 14 Nov 2019 01:46:37 -0800 Subject: [PATCH] Reimplement android.os.SystemProperties in terms of libc facilities A subsequent CL will implement a new prop_info based properties API on top of this CL. Test: boots Test: atest FrameworksCoreSystemPropertiesTests Bug: 140788621 Change-Id: Id8dd02fff7b1c0815a27ab1dfdde1700447a414c --- core/java/android/os/SystemProperties.java | 45 ++++-- core/jni/android_os_SystemProperties.cpp | 154 +++++++++++++-------- 2 files changed, 127 insertions(+), 72 deletions(-) diff --git a/core/java/android/os/SystemProperties.java b/core/java/android/os/SystemProperties.java index 8b0ffe155fc37..2b4f6f867e038 100644 --- a/core/java/android/os/SystemProperties.java +++ b/core/java/android/os/SystemProperties.java @@ -26,6 +26,8 @@ import android.util.MutableInt; import com.android.internal.annotations.GuardedBy; +import dalvik.annotation.optimization.FastNative; + import libcore.util.HexEncoding; import java.nio.charset.StandardCharsets; @@ -35,6 +37,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; + /** * Gives access to the system properties store. The system properties * store contains a list of string key-value pairs. @@ -93,18 +96,32 @@ public class SystemProperties { } } + // The one-argument version of native_get used to be a regular native function. Nowadays, + // we use the two-argument form of native_get all the time, but we can't just delete the + // one-argument overload: apps use it via reflection, as the UnsupportedAppUsage annotation + // indicates. Let's just live with having a Java function with a very unusual name. @UnsupportedAppUsage - private static native String native_get(String key); + private static String native_get(String key) { + return native_get(key, ""); + } + + @FastNative @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) private static native String native_get(String key, String def); + @FastNative @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) private static native int native_get_int(String key, int def); + @FastNative @UnsupportedAppUsage private static native long native_get_long(String key, long def); + @FastNative @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) private static native boolean native_get_boolean(String key, boolean def); + + // _NOT_ FastNative: native_set performs IPC and can block @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) private static native void native_set(String key, String def); + @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) private static native void native_add_change_callback(); private static native void native_report_sysprop_change(); @@ -229,25 +246,27 @@ public class SystemProperties { @SuppressWarnings("unused") // Called from native code. private static void callChangeCallbacks() { + ArrayList callbacks = null; synchronized (sChangeCallbacks) { //Log.i("foo", "Calling " + sChangeCallbacks.size() + " change callbacks!"); if (sChangeCallbacks.size() == 0) { return; } - ArrayList callbacks = new ArrayList(sChangeCallbacks); - final long token = Binder.clearCallingIdentity(); - try { - for (int i = 0; i < callbacks.size(); i++) { - try { - callbacks.get(i).run(); - } catch (Throwable t) { - Log.wtf(TAG, "Exception in SystemProperties change callback", t); - // Ignore and try to go on. - } + callbacks = new ArrayList(sChangeCallbacks); + } + final long token = Binder.clearCallingIdentity(); + try { + for (int i = 0; i < callbacks.size(); i++) { + try { + callbacks.get(i).run(); + } catch (Throwable t) { + // Ignore and try to go on. Don't use wtf here: that + // will cause the process to exit on some builds and break tests. + Log.e(TAG, "Exception in SystemProperties change callback", t); } - } finally { - Binder.restoreCallingIdentity(token); } + } finally { + Binder.restoreCallingIdentity(token); } } diff --git a/core/jni/android_os_SystemProperties.cpp b/core/jni/android_os_SystemProperties.cpp index 87f498a710c10..07617d600dbcf 100644 --- a/core/jni/android_os_SystemProperties.cpp +++ b/core/jni/android_os_SystemProperties.cpp @@ -17,9 +17,13 @@ #define LOG_TAG "SysPropJNI" +#include +#include + #include "android-base/logging.h" +#include "android-base/parsebool.h" +#include "android-base/parseint.h" #include "android-base/properties.h" -#include "cutils/properties.h" #include "utils/misc.h" #include #include "jni.h" @@ -28,86 +32,120 @@ #include #include -namespace android -{ +#if defined(__BIONIC__) +# include +#else +struct prop_info; +#endif +namespace android { namespace { -template -T ConvertKeyAndForward(JNIEnv *env, jstring keyJ, T defJ, Handler handler) { - std::string key; - { - // Scope the String access. If the handler can throw an exception, - // releasing the string characters late would trigger an abort. - ScopedUtfChars key_utf(env, keyJ); - if (key_utf.c_str() == nullptr) { - return defJ; - } - key = key_utf.c_str(); // This will make a copy, but we can't avoid - // with the existing interface in - // android::base. - } - return handler(key, defJ); +template +void ReadProperty(const prop_info* prop, Functor&& functor) +{ +#if defined(__BIONIC__) + auto thunk = [](void* cookie, + const char* /*name*/, + const char* value, + uint32_t /*serial*/) { + std::forward(*static_cast(cookie))(value); + }; + __system_property_read_callback(prop, thunk, &functor); +#else + LOG(FATAL) << "fast property access supported only on device"; +#endif } -jstring SystemProperties_getSS(JNIEnv *env, jclass clazz, jstring keyJ, +template +void ReadProperty(JNIEnv* env, jstring keyJ, Functor&& functor) +{ + ScopedUtfChars key(env, keyJ); + if (!key.c_str()) { + return; + } +#if defined(__BIONIC__) + const prop_info* prop = __system_property_find(key.c_str()); + if (!prop) { + return; + } + ReadProperty(prop, std::forward(functor)); +#else + std::forward(functor)( + android::base::GetProperty(key.c_str(), "").c_str()); +#endif +} + +jstring SystemProperties_getSS(JNIEnv* env, jclass clazz, jstring keyJ, jstring defJ) { - // Using ConvertKeyAndForward is sub-optimal for copying the key string, - // but improves reuse and reasoning over code. - auto handler = [&](const std::string& key, jstring defJ) { - std::string prop_val = android::base::GetProperty(key, ""); - if (!prop_val.empty()) { - return env->NewStringUTF(prop_val.c_str()); - }; - if (defJ != nullptr) { - return defJ; + jstring ret = defJ; + ReadProperty(env, keyJ, [&](const char* value) { + if (value[0]) { + ret = env->NewStringUTF(value); } - // This function is specified to never return null (or have an - // exception pending). - return env->NewStringUTF(""); - }; - return ConvertKeyAndForward(env, keyJ, defJ, handler); -} - -jstring SystemProperties_getS(JNIEnv *env, jclass clazz, jstring keyJ) -{ - return SystemProperties_getSS(env, clazz, keyJ, nullptr); + }); + if (ret == nullptr && !env->ExceptionCheck()) { + ret = env->NewStringUTF(""); // Legacy behavior + } + return ret; } template T SystemProperties_get_integral(JNIEnv *env, jclass, jstring keyJ, T defJ) { - auto handler = [](const std::string& key, T defV) { - return android::base::GetIntProperty(key, defV); - }; - return ConvertKeyAndForward(env, keyJ, defJ, handler); + T ret = defJ; + ReadProperty(env, keyJ, [&](const char* value) { + android::base::ParseInt(value, &ret); + }); + return ret; } jboolean SystemProperties_get_boolean(JNIEnv *env, jclass, jstring keyJ, jboolean defJ) { - auto handler = [](const std::string& key, jboolean defV) -> jboolean { - bool result = android::base::GetBoolProperty(key, defV); - return result ? JNI_TRUE : JNI_FALSE; - }; - return ConvertKeyAndForward(env, keyJ, defJ, handler); + using android::base::ParseBoolResult; + ParseBoolResult parseResult; + ReadProperty(env, keyJ, [&](const char* value) { + parseResult = android::base::ParseBool(value); + }); + jboolean ret; + switch (parseResult) { + case ParseBoolResult::kError: + ret = defJ; + break; + case ParseBoolResult::kFalse: + ret = JNI_FALSE; + break; + case ParseBoolResult::kTrue: + ret = JNI_TRUE; + break; + } + return ret; } void SystemProperties_set(JNIEnv *env, jobject clazz, jstring keyJ, jstring valJ) { - auto handler = [&](const std::string& key, bool) { - std::string val; - if (valJ != nullptr) { - ScopedUtfChars key_utf(env, valJ); - val = key_utf.c_str(); + ScopedUtfChars key(env, keyJ); + if (!key.c_str()) { + return; + } + std::optional value; + if (valJ != nullptr) { + value.emplace(env, valJ); + if (!value->c_str()) { + return; } - return android::base::SetProperty(key, val); - }; - if (!ConvertKeyAndForward(env, keyJ, true, handler)) { - // Must have been a failure in SetProperty. + } + bool success; +#if defined(__BIONIC__) + success = !__system_property_set(key.c_str(), value ? value->c_str() : ""); +#else + success = android::base::SetProperty(key.c_str(), value ? value->c_str() : ""); +#endif + if (!success) { jniThrowException(env, "java/lang/RuntimeException", "failed to set system property (check logcat for reason)"); } @@ -157,8 +195,6 @@ void SystemProperties_report_sysprop_change(JNIEnv /**env*/, jobject /*clazz*/) int register_android_os_SystemProperties(JNIEnv *env) { const JNINativeMethod method_table[] = { - { "native_get", "(Ljava/lang/String;)Ljava/lang/String;", - (void*) SystemProperties_getS }, { "native_get", "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;", (void*) SystemProperties_getSS }, @@ -179,4 +215,4 @@ int register_android_os_SystemProperties(JNIEnv *env) method_table, NELEM(method_table)); } -}; +} // namespace android