diff options
60 files changed, 2229 insertions, 514 deletions
diff --git a/cmds/atrace/atrace.rc b/cmds/atrace/atrace.rc index 8da1352cca..97ccda4338 100644 --- a/cmds/atrace/atrace.rc +++ b/cmds/atrace/atrace.rc @@ -105,7 +105,6 @@ on late-init chmod 0755 /sys/kernel/debug/tracing/events/sched/sched_cpu_hotplug chmod 0755 /sys/kernel/debug/tracing/events/sched/sched_pi_setprio chmod 0755 /sys/kernel/debug/tracing/events/sched/sched_process_exit - chmod 0755 /sys/kernel/debug/tracing/events/sched/sched_process_free chmod 0755 /sys/kernel/debug/tracing/events/sched/sched_switch chmod 0755 /sys/kernel/debug/tracing/events/sched/sched_wakeup chmod 0755 /sys/kernel/debug/tracing/events/sched/sched_wakeup_new @@ -245,7 +244,6 @@ on late-init chmod 0755 /sys/kernel/tracing/events/sched/sched_cpu_hotplug chmod 0755 /sys/kernel/tracing/events/sched/sched_pi_setprio chmod 0755 /sys/kernel/tracing/events/sched/sched_process_exit - chmod 0755 /sys/kernel/tracing/events/sched/sched_process_free chmod 0755 /sys/kernel/tracing/events/sched/sched_switch chmod 0755 /sys/kernel/tracing/events/sched/sched_wakeup chmod 0755 /sys/kernel/tracing/events/sched/sched_wakeup_new diff --git a/cmds/installd/Android.bp b/cmds/installd/Android.bp index faa84854ba..00babc3346 100644 --- a/cmds/installd/Android.bp +++ b/cmds/installd/Android.bp @@ -28,6 +28,7 @@ cc_defaults { "dexopt.cpp", "execv_helper.cpp", "globals.cpp", + "restorable_file.cpp", "run_dex2oat.cpp", "unique_file.cpp", "utils.cpp", @@ -80,7 +81,7 @@ cc_defaults { "-cert-err58-cpp", ], tidy_flags: [ - "-warnings-as-errors=clang-analyzer-security*,cert-*" + "-warnings-as-errors=clang-analyzer-security*,cert-*", ], } @@ -132,7 +133,10 @@ cc_test_host { "unique_file.cpp", "execv_helper.cpp", ], - cflags: ["-Wall", "-Werror"], + cflags: [ + "-Wall", + "-Werror", + ], shared_libs: [ "libbase", "server_configurable_flags", @@ -170,7 +174,7 @@ cc_binary { // Needs to be wherever installd is as it's execed by // installd. - required: [ "migrate_legacy_obb_data.sh" ], + required: ["migrate_legacy_obb_data.sh"], } // OTA chroot tool @@ -194,7 +198,7 @@ cc_binary { "libutils", ], required: [ - "apexd" + "apexd", ], } @@ -213,7 +217,7 @@ cc_library_static { name: "libotapreoptparameters", cflags: [ "-Wall", - "-Werror" + "-Werror", ], srcs: ["otapreopt_parameters.cpp"], @@ -237,7 +241,7 @@ cc_binary { name: "otapreopt", cflags: [ "-Wall", - "-Werror" + "-Werror", ], srcs: [ @@ -246,6 +250,7 @@ cc_binary { "globals.cpp", "otapreopt.cpp", "otapreopt_utils.cpp", + "restorable_file.cpp", "run_dex2oat.cpp", "unique_file.cpp", "utils.cpp", @@ -296,5 +301,5 @@ sh_binary { // Script to migrate legacy obb data. sh_binary { name: "migrate_legacy_obb_data.sh", - src: "migrate_legacy_obb_data.sh" + src: "migrate_legacy_obb_data.sh", } diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 2bcf2d473d..b6f42ad172 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -58,9 +58,10 @@ #include "dexopt_return_codes.h" #include "execv_helper.h" #include "globals.h" -#include "installd_deps.h" #include "installd_constants.h" +#include "installd_deps.h" #include "otapreopt_utils.h" +#include "restorable_file.h" #include "run_dex2oat.h" #include "unique_file.h" #include "utils.h" @@ -309,12 +310,6 @@ static bool IsBootClassPathProfilingEnable() { return profile_boot_class_path == "true"; } -static void UnlinkIgnoreResult(const std::string& path) { - if (unlink(path.c_str()) < 0) { - PLOG(ERROR) << "Failed to unlink " << path; - } -} - /* * Whether dexopt should use a swap file when compiling an APK. * @@ -988,42 +983,34 @@ static bool create_oat_out_path(const char* apk_path, const char* instruction_se } // (re)Creates the app image if needed. -UniqueFile maybe_open_app_image(const std::string& out_oat_path, - bool generate_app_image, bool is_public, int uid, bool is_secondary_dex) { - +RestorableFile maybe_open_app_image(const std::string& out_oat_path, bool generate_app_image, + bool is_public, int uid, bool is_secondary_dex) { const std::string image_path = create_image_filename(out_oat_path); if (image_path.empty()) { // Happens when the out_oat_path has an unknown extension. - return UniqueFile(); + return RestorableFile(); } - // In case there is a stale image, remove it now. Ignore any error. - unlink(image_path.c_str()); - // Not enabled, exit. if (!generate_app_image) { - return UniqueFile(); + RestorableFile::RemoveAllFiles(image_path); + return RestorableFile(); } std::string app_image_format = GetProperty("dalvik.vm.appimageformat", ""); if (app_image_format.empty()) { - return UniqueFile(); + RestorableFile::RemoveAllFiles(image_path); + return RestorableFile(); } - // Recreate is true since we do not want to modify a mapped image. If the app is - // already running and we modify the image file, it can cause crashes (b/27493510). - UniqueFile image_file( - open_output_file(image_path.c_str(), true /*recreate*/, 0600 /*permissions*/), - image_path, - UnlinkIgnoreResult); + // If the app is already running and we modify the image file, it can cause crashes + // (b/27493510). + RestorableFile image_file = RestorableFile::CreateWritableFile(image_path, + /*permissions*/ 0600); if (image_file.fd() < 0) { // Could not create application image file. Go on since we can compile without it. LOG(ERROR) << "installd could not create '" << image_path << "' for image file during dexopt"; - // If we have a valid image file path but no image fd, explicitly erase the image file. - if (unlink(image_path.c_str()) < 0) { - if (errno != ENOENT) { - PLOG(ERROR) << "Couldn't unlink image file " << image_path; - } - } + // If we have a valid image file path but cannot create tmp file, reset it. + image_file.reset(); } else if (!set_permissions_and_ownership( image_file.fd(), is_public, uid, image_path.c_str(), is_secondary_dex)) { ALOGE("installd cannot set owner '%s' for image during dexopt\n", image_path.c_str()); @@ -1097,9 +1084,9 @@ UniqueFile maybe_open_reference_profile(const std::string& pkgname, // Opens the vdex files and assigns the input fd to in_vdex_wrapper and the output fd to // out_vdex_wrapper. Returns true for success or false in case of errors. bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, int dexopt_needed, - const char* instruction_set, bool is_public, int uid, bool is_secondary_dex, - bool profile_guided, UniqueFile* in_vdex_wrapper, - UniqueFile* out_vdex_wrapper) { + const char* instruction_set, bool is_public, int uid, + bool is_secondary_dex, bool profile_guided, + UniqueFile* in_vdex_wrapper, RestorableFile* out_vdex_wrapper) { CHECK(in_vdex_wrapper != nullptr); CHECK(out_vdex_wrapper != nullptr); // Open the existing VDEX. We do this before creating the new output VDEX, which will @@ -1114,6 +1101,14 @@ bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, return false; } + // Create work file first. All files will be deleted when it fails. + *out_vdex_wrapper = RestorableFile::CreateWritableFile(out_vdex_path_str, + /*permissions*/ 0644); + if (out_vdex_wrapper->fd() < 0) { + ALOGE("installd cannot open vdex '%s' during dexopt\n", out_vdex_path_str.c_str()); + return false; + } + bool update_vdex_in_place = false; if (dexopt_action != DEX2OAT_FROM_SCRATCH) { // Open the possibly existing vdex. If none exist, we pass -1 to dex2oat for input-vdex-fd. @@ -1145,41 +1140,19 @@ bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, (dexopt_action == DEX2OAT_FOR_BOOT_IMAGE) && !profile_guided; if (update_vdex_in_place) { + // dex2oat marks it invalid anyway. So delete it and set work file fd. + unlink(in_vdex_path_str.c_str()); // Open the file read-write to be able to update it. - in_vdex_wrapper->reset(open(in_vdex_path_str.c_str(), O_RDWR, 0), - in_vdex_path_str); - if (in_vdex_wrapper->fd() == -1) { - // If we failed to open the file, we cannot update it in place. - update_vdex_in_place = false; - } + in_vdex_wrapper->reset(out_vdex_wrapper->fd(), in_vdex_path_str); + // Disable auto close for the in wrapper fd (it will be done when destructing the out + // wrapper). + in_vdex_wrapper->DisableAutoClose(); } else { in_vdex_wrapper->reset(open(in_vdex_path_str.c_str(), O_RDONLY, 0), in_vdex_path_str); } } - // If we are updating the vdex in place, we do not need to recreate a vdex, - // and can use the same existing one. - if (update_vdex_in_place) { - // We unlink the file in case the invocation of dex2oat fails, to ensure we don't - // have bogus stale vdex files. - out_vdex_wrapper->reset( - in_vdex_wrapper->fd(), - out_vdex_path_str, - UnlinkIgnoreResult); - // Disable auto close for the in wrapper fd (it will be done when destructing the out - // wrapper). - in_vdex_wrapper->DisableAutoClose(); - } else { - out_vdex_wrapper->reset( - open_output_file(out_vdex_path_str.c_str(), /*recreate*/true, /*permissions*/0644), - out_vdex_path_str, - UnlinkIgnoreResult); - if (out_vdex_wrapper->fd() < 0) { - ALOGE("installd cannot open vdex'%s' during dexopt\n", out_vdex_path_str.c_str()); - return false; - } - } if (!set_permissions_and_ownership(out_vdex_wrapper->fd(), is_public, uid, out_vdex_path_str.c_str(), is_secondary_dex)) { ALOGE("installd cannot set owner '%s' for vdex during dexopt\n", out_vdex_path_str.c_str()); @@ -1191,16 +1164,13 @@ bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, } // Opens the output oat file for the given apk. -UniqueFile open_oat_out_file(const char* apk_path, const char* oat_dir, - bool is_public, int uid, const char* instruction_set, bool is_secondary_dex) { +RestorableFile open_oat_out_file(const char* apk_path, const char* oat_dir, bool is_public, int uid, + const char* instruction_set, bool is_secondary_dex) { char out_oat_path[PKG_PATH_MAX]; if (!create_oat_out_path(apk_path, instruction_set, oat_dir, is_secondary_dex, out_oat_path)) { - return UniqueFile(); + return RestorableFile(); } - UniqueFile oat( - open_output_file(out_oat_path, /*recreate*/true, /*permissions*/0644), - out_oat_path, - UnlinkIgnoreResult); + RestorableFile oat = RestorableFile::CreateWritableFile(out_oat_path, /*permissions*/ 0644); if (oat.fd() < 0) { PLOG(ERROR) << "installd cannot open output during dexopt" << out_oat_path; } else if (!set_permissions_and_ownership( @@ -1839,6 +1809,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins if (sec_dex_result == kSecondaryDexOptProcessOk) { oat_dir = oat_dir_str.c_str(); if (dexopt_needed == NO_DEXOPT_NEEDED) { + *completed = true; return 0; // Nothing to do, report success. } } else if (sec_dex_result == kSecondaryDexOptProcessCancelled) { @@ -1874,8 +1845,8 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins } // Create the output OAT file. - UniqueFile out_oat = open_oat_out_file(dex_path, oat_dir, is_public, uid, - instruction_set, is_secondary_dex); + RestorableFile out_oat = + open_oat_out_file(dex_path, oat_dir, is_public, uid, instruction_set, is_secondary_dex); if (out_oat.fd() < 0) { *error_msg = "Could not open out oat file."; return -1; @@ -1883,7 +1854,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins // Open vdex files. UniqueFile in_vdex; - UniqueFile out_vdex; + RestorableFile out_vdex; if (!open_vdex_files_for_dex2oat(dex_path, out_oat.path().c_str(), dexopt_needed, instruction_set, is_public, uid, is_secondary_dex, profile_guided, &in_vdex, &out_vdex)) { @@ -1919,8 +1890,8 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins } // Create the app image file if needed. - UniqueFile out_image = maybe_open_app_image( - out_oat.path(), generate_app_image, is_public, uid, is_secondary_dex); + RestorableFile out_image = maybe_open_app_image(out_oat.path(), generate_app_image, is_public, + uid, is_secondary_dex); UniqueFile dex_metadata; if (dex_metadata_path != nullptr) { @@ -1953,30 +1924,18 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins LOG(VERBOSE) << "DexInv: --- BEGIN '" << dex_path << "' ---"; RunDex2Oat runner(dex2oat_bin, execv_helper.get()); - runner.Initialize(out_oat, - out_vdex, - out_image, - in_dex, - in_vdex, - dex_metadata, - reference_profile, - class_loader_context, - join_fds(context_input_fds), - swap_fd.get(), - instruction_set, - compiler_filter, - debuggable, - boot_complete, - for_restore, - target_sdk_version, - enable_hidden_api_checks, - generate_compact_dex, - use_jitzygote_image, + runner.Initialize(out_oat.GetUniqueFile(), out_vdex.GetUniqueFile(), out_image.GetUniqueFile(), + in_dex, in_vdex, dex_metadata, reference_profile, class_loader_context, + join_fds(context_input_fds), swap_fd.get(), instruction_set, compiler_filter, + debuggable, boot_complete, for_restore, target_sdk_version, + enable_hidden_api_checks, generate_compact_dex, use_jitzygote_image, compilation_reason); bool cancelled = false; pid_t pid = dexopt_status_->check_cancellation_and_fork(&cancelled); if (cancelled) { + *completed = false; + reference_profile.DisableCleanup(); return 0; } if (pid == 0) { @@ -2004,6 +1963,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins LOG(VERBOSE) << "DexInv: --- END '" << dex_path << "' --- cancelled"; // cancelled, not an error *completed = false; + reference_profile.DisableCleanup(); return 0; } LOG(VERBOSE) << "DexInv: --- END '" << dex_path << "' --- status=0x" @@ -2013,13 +1973,42 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins } } - // TODO(b/156537504) Implement SWAP of completed files - // We've been successful, don't delete output. - out_oat.DisableCleanup(); - out_vdex.DisableCleanup(); - out_image.DisableCleanup(); + // dex2oat ran successfully, so profile is safe to keep. reference_profile.DisableCleanup(); + // We've been successful, commit work files. + // If committing (=renaming tmp to regular) fails, try to restore backup files. + // If restoring fails as well, as a last resort, remove all files. + if (!out_oat.CreateBackupFile() || !out_vdex.CreateBackupFile() || + !out_image.CreateBackupFile()) { + // Renaming failure can mean that the original file may not be accessible from installd. + LOG(ERROR) << "Cannot create backup file from existing file, file in wrong state?" + << ", out_oat:" << out_oat.path() << " ,out_vdex:" << out_vdex.path() + << " ,out_image:" << out_image.path(); + out_oat.ResetAndRemoveAllFiles(); + out_vdex.ResetAndRemoveAllFiles(); + out_image.ResetAndRemoveAllFiles(); + return -1; + } + if (!out_oat.CommitWorkFile() || !out_vdex.CommitWorkFile() || !out_image.CommitWorkFile()) { + LOG(ERROR) << "Cannot commit, out_oat:" << out_oat.path() + << " ,out_vdex:" << out_vdex.path() << " ,out_image:" << out_image.path(); + if (!out_oat.RestoreBackupFile() || !out_vdex.RestoreBackupFile() || + !out_image.RestoreBackupFile()) { + LOG(ERROR) << "Cannot cancel commit, out_oat:" << out_oat.path() + << " ,out_vdex:" << out_vdex.path() << " ,out_image:" << out_image.path(); + // Restoring failed. + out_oat.ResetAndRemoveAllFiles(); + out_vdex.ResetAndRemoveAllFiles(); + out_image.ResetAndRemoveAllFiles(); + } + return -1; + } + // Now remove remaining backup files. + out_oat.RemoveBackupFile(); + out_vdex.RemoveBackupFile(); + out_image.RemoveBackupFile(); + *completed = true; return 0; } diff --git a/cmds/installd/restorable_file.cpp b/cmds/installd/restorable_file.cpp new file mode 100644 index 0000000000..fd54a232d5 --- /dev/null +++ b/cmds/installd/restorable_file.cpp @@ -0,0 +1,161 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "restorable_file.h" + +#include <string> + +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <unistd.h> + +#include <android-base/logging.h> +#include <android-base/stringprintf.h> + +namespace { + +constexpr char kTmpFileSuffix[] = ".tmp"; +constexpr char kBackupFileSuffix[] = ".backup"; + +std::string GetTmpFilePath(const std::string& path) { + return android::base::StringPrintf("%s%s", path.c_str(), kTmpFileSuffix); +} + +std::string GetBackupFilePath(const std::string& path) { + return android::base::StringPrintf("%s%s", path.c_str(), kBackupFileSuffix); +} + +void UnlinkPossiblyNonExistingFile(const std::string& path) { + if (unlink(path.c_str()) < 0) { + if (errno != ENOENT && errno != EROFS) { // EROFS reported even if it does not exist. + PLOG(ERROR) << "Cannot unlink: " << path; + } + } +} + +// Check if file for the given path exists +bool FileExists(const std::string& path) { + struct stat st; + return ::stat(path.c_str(), &st) == 0; +} + +} // namespace + +namespace android { +namespace installd { + +RestorableFile::RestorableFile() : RestorableFile(-1, "") {} + +RestorableFile::RestorableFile(int value, const std::string& path) : unique_file_(value, path) { + // As cleanup is null, this does not make much difference but use unique_file_ only for closing + // tmp file. + unique_file_.DisableCleanup(); +} + +RestorableFile::~RestorableFile() { + reset(); +} + +void RestorableFile::reset() { + // need to copy before reset clears it. + std::string path(unique_file_.path()); + unique_file_.reset(); + if (!path.empty()) { + UnlinkPossiblyNonExistingFile(GetTmpFilePath(path)); + } +} + +bool RestorableFile::CreateBackupFile() { + if (path().empty() || !FileExists(path())) { + return true; + } + std::string backup = GetBackupFilePath(path()); + UnlinkPossiblyNonExistingFile(backup); + if (rename(path().c_str(), backup.c_str()) < 0) { + PLOG(ERROR) << "Cannot rename " << path() << " to " << backup; + return false; + } + return true; +} + +bool RestorableFile::CommitWorkFile() { + std::string path(unique_file_.path()); + // Keep the path with Commit for debugging purpose. + unique_file_.reset(-1, path); + if (!path.empty()) { + if (rename(GetTmpFilePath(path).c_str(), path.c_str()) < 0) { + PLOG(ERROR) << "Cannot rename " << GetTmpFilePath(path) << " to " << path; + // Remove both files as renaming can fail due to the original file as well. + UnlinkPossiblyNonExistingFile(path); + UnlinkPossiblyNonExistingFile(GetTmpFilePath(path)); + return false; + } + } + + return true; +} + +bool RestorableFile::RestoreBackupFile() { + std::string backup = GetBackupFilePath(path()); + if (path().empty() || !FileExists(backup)) { + return true; + } + UnlinkPossiblyNonExistingFile(path()); + if (rename(backup.c_str(), path().c_str()) < 0) { + PLOG(ERROR) << "Cannot rename " << backup << " to " << path(); + return false; + } + return true; +} + +void RestorableFile::RemoveBackupFile() { + UnlinkPossiblyNonExistingFile(GetBackupFilePath(path())); +} + +const UniqueFile& RestorableFile::GetUniqueFile() const { + return unique_file_; +} + +void RestorableFile::ResetAndRemoveAllFiles() { + std::string path(unique_file_.path()); + reset(); + RemoveAllFiles(path); +} + +RestorableFile RestorableFile::CreateWritableFile(const std::string& path, int permissions) { + std::string tmp_file_path = GetTmpFilePath(path); + // If old tmp file exists, delete it. + UnlinkPossiblyNonExistingFile(tmp_file_path); + int fd = -1; + if (!path.empty()) { + fd = open(tmp_file_path.c_str(), O_RDWR | O_CREAT, permissions); + if (fd < 0) { + PLOG(ERROR) << "Cannot create file: " << tmp_file_path; + } + } + RestorableFile rf(fd, path); + return rf; +} + +void RestorableFile::RemoveAllFiles(const std::string& path) { + UnlinkPossiblyNonExistingFile(GetTmpFilePath(path)); + UnlinkPossiblyNonExistingFile(GetBackupFilePath(path)); + UnlinkPossiblyNonExistingFile(path); +} + +} // namespace installd +} // namespace android diff --git a/cmds/installd/restorable_file.h b/cmds/installd/restorable_file.h new file mode 100644 index 0000000000..eda229241a --- /dev/null +++ b/cmds/installd/restorable_file.h @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_INSTALLD_RESTORABLE_FILE_H +#define ANDROID_INSTALLD_RESTORABLE_FILE_H + +#include <functional> +#include <string> + +#include "unique_file.h" + +namespace android { +namespace installd { + +// This is a file abstraction which allows restoring to the original file while temporary work +// file is updated. +// +// Typical flow for this API will be: +// RestorableFile rf = RestorableFile::CreateWritableFile(...) +// write to file using file descriptor acquired from: rf.fd() +// Make work file into a regular file with: rf.CommitWorkFile() +// Or throw away the work file by destroying the instance without calling CommitWorkFile(). +// The temporary work file is closed / removed when an instance is destroyed without calling +// CommitWorkFile(). The original file, if CommitWorkFile() is not called, will be kept. +// +// For safer restoration of original file when commit fails, following 3 steps can be taken: +// 1. CreateBackupFile(): This renames an existing regular file into a separate backup file. +// 2. CommitWorkFile(): Rename the work file into the regular file. +// 3. RemoveBackupFile(): Removes the backup file +// If CommitWorkFile fails, client can call RestoreBackupFile() which will restore regular file from +// the backup. +class RestorableFile { +public: + // Creates invalid instance with no fd (=-1) and empty path. + RestorableFile(); + RestorableFile(RestorableFile&& other) = default; + ~RestorableFile(); + + // Passes all contents of other file into the current file. + // Files kept for the current file will be either deleted or committed depending on + // CommitWorkFile() and DisableCleanUp() calls made before this. + RestorableFile& operator=(RestorableFile&& other) = default; + + // Gets file descriptor for backing work (=temporary) file. If work file does not exist, it will + // return -1. + int fd() const { return unique_file_.fd(); } + + // Gets the path name for the regular file (not temporary file). + const std::string& path() const { return unique_file_.path(); } + + // Closes work file, deletes it and resets all internal states into default states. + void reset(); + + // Closes work file and closes all files including work file, backup file and regular file. + void ResetAndRemoveAllFiles(); + + // Creates a backup file by renaming existing regular file. This will return false if renaming + // fails. If regular file for renaming does not exist, it will return true. + bool CreateBackupFile(); + + // Closes existing work file and makes it a regular file. + // Note that the work file is closed and fd() will return -1 after this. path() will still + // return the original path. + // This will return false when committing fails (=cannot rename). Both the regular file and tmp + // file will be deleted when it fails. + bool CommitWorkFile(); + + // Cancels the commit and restores the backup file into the regular one. If renaming fails, + // it will return false. This returns true if the backup file does not exist. + bool RestoreBackupFile(); + + // Removes the backup file. + void RemoveBackupFile(); + + // Gets UniqueFile with the same path and fd() pointing to the work file. + const UniqueFile& GetUniqueFile() const; + + // Creates writable RestorableFile. This involves creating tmp file for writing. + static RestorableFile CreateWritableFile(const std::string& path, int permissions); + + // Removes the specified file together with tmp file generated as RestorableFile. + static void RemoveAllFiles(const std::string& path); + +private: + RestorableFile(int value, const std::string& path); + + // Used as a storage for work file fd and path string. + UniqueFile unique_file_; +}; + +} // namespace installd +} // namespace android + +#endif // ANDROID_INSTALLD_RESTORABLE_FILE_H diff --git a/cmds/installd/tests/Android.bp b/cmds/installd/tests/Android.bp index 4cde7e3fb7..51f7716d3a 100644 --- a/cmds/installd/tests/Android.bp +++ b/cmds/installd/tests/Android.bp @@ -188,3 +188,23 @@ cc_test { "libotapreoptparameters", ], } + +cc_test { + name: "installd_file_test", + test_suites: ["device-tests"], + clang: true, + srcs: ["installd_file_test.cpp"], + cflags: [ + "-Wall", + "-Werror", + ], + shared_libs: [ + "libbase", + "libcutils", + "libutils", + ], + static_libs: [ + "libinstalld", + "liblog", + ], +} diff --git a/cmds/installd/tests/installd_file_test.cpp b/cmds/installd/tests/installd_file_test.cpp new file mode 100644 index 0000000000..00fb30875f --- /dev/null +++ b/cmds/installd/tests/installd_file_test.cpp @@ -0,0 +1,521 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <android-base/file.h> +#include <android-base/logging.h> +#include <android-base/stringprintf.h> +#include <gtest/gtest.h> +#include <log/log.h> +#include <stdlib.h> +#include <string.h> + +#include "restorable_file.h" +#include "unique_file.h" +#include "utils.h" + +#undef LOG_TAG +#define LOG_TAG "installd_file_test" + +namespace { + +constexpr char kFileTestDir[] = "/data/local/tmp/installd_file_test_data"; +constexpr char kTmpFileSuffix[] = ".tmp"; +constexpr char kBackupFileSuffix[] = ".backup"; + +void UnlinkWithAssert(const std::string& path) { + ASSERT_EQ(0, unlink(path.c_str())); +} + +} // namespace + +namespace android { +namespace installd { + +// Add these as macros as functions make it hard to tell where the failure has happened. +#define ASSERT_FILE_NOT_EXISTING(path) \ + { \ + struct stat st; \ + ASSERT_NE(0, ::stat(path.c_str(), &st)); \ + } +#define ASSERT_FILE_EXISTING(path) \ + { \ + struct stat st; \ + ASSERT_EQ(0, ::stat(path.c_str(), &st)); \ + } +#define ASSERT_FILE_CONTENT(path, expectedContent) ASSERT_EQ(expectedContent, ReadTestFile(path)) +#define ASSERT_FILE_OPEN(path, fd) \ + { \ + fd = open(path.c_str(), O_RDWR); \ + ASSERT_TRUE(fd >= 0); \ + } +#define ASSERT_WRITE_TO_FD(fd, content) \ + ASSERT_TRUE(android::base::WriteStringToFd(content, android::base::borrowed_fd(fd))) + +class FileTest : public testing::Test { +protected: + virtual void SetUp() { + setenv("ANDROID_LOG_TAGS", "*:v", 1); + android::base::InitLogging(nullptr); + + ASSERT_EQ(0, create_dir_if_needed(kFileTestDir, 0777)); + } + + virtual void TearDown() { + system(android::base::StringPrintf("rm -rf %s", kFileTestDir).c_str()); + } + + std::string GetTestFilePath(const std::string& fileName) { + return android::base::StringPrintf("%s/%s", kFileTestDir, fileName.c_str()); + } + + void CreateTestFileWithContents(const std::string& path, const std::string& content) { + ALOGI("CreateTestFileWithContents:%s", path.c_str()); + ASSERT_TRUE(android::base::WriteStringToFile(content, path)); + } + + std::string GetTestName() { + std::string name(testing::UnitTest::GetInstance()->current_test_info()->name()); + return name; + } + + std::string ReadTestFile(const std::string& path) { + std::string content; + bool r = android::base::ReadFileToString(path, &content); + if (!r) { + PLOG(ERROR) << "Cannot read file:" << path; + } + return content; + } +}; + +TEST_F(FileTest, TestUniqueFileMoveConstruction) { + const int fd = 101; + std::string testFile = GetTestFilePath(GetTestName()); + UniqueFile uf1(fd, testFile); + uf1.DisableAutoClose(); + + UniqueFile uf2(std::move(uf1)); + + ASSERT_EQ(fd, uf2.fd()); + ASSERT_EQ(testFile, uf2.path()); +} + +TEST_F(FileTest, TestUniqueFileAssignment) { + const int fd1 = 101; + const int fd2 = 102; + std::string testFile1 = GetTestFilePath(GetTestName()); + std::string testFile2 = GetTestFilePath(GetTestName() + "2"); + + UniqueFile uf1(fd1, testFile1); + uf1.DisableAutoClose(); + + UniqueFile uf2(fd2, testFile2); + uf2.DisableAutoClose(); + + ASSERT_EQ(fd2, uf2.fd()); + ASSERT_EQ(testFile2, uf2.path()); + + uf2 = std::move(uf1); + + ASSERT_EQ(fd1, uf2.fd()); + ASSERT_EQ(testFile1, uf2.path()); +} + +TEST_F(FileTest, TestUniqueFileCleanup) { + std::string testFile = GetTestFilePath(GetTestName()); + CreateTestFileWithContents(testFile, "OriginalContent"); + + int fd; + ASSERT_FILE_OPEN(testFile, fd); + + { UniqueFile uf = UniqueFile(fd, testFile, UnlinkWithAssert); } + + ASSERT_FILE_NOT_EXISTING(testFile); +} + +TEST_F(FileTest, TestUniqueFileNoCleanup) { + std::string testFile = GetTestFilePath(GetTestName()); + CreateTestFileWithContents(testFile, "OriginalContent"); + + int fd; + ASSERT_FILE_OPEN(testFile, fd); + + { + UniqueFile uf = UniqueFile(fd, testFile, UnlinkWithAssert); + uf.DisableCleanup(); + } + + ASSERT_FILE_CONTENT(testFile, "OriginalContent"); +} + +TEST_F(FileTest, TestUniqueFileFd) { + std::string testFile = GetTestFilePath(GetTestName()); + CreateTestFileWithContents(testFile, "OriginalContent"); + + int fd; + ASSERT_FILE_OPEN(testFile, fd); + + UniqueFile uf(fd, testFile, UnlinkWithAssert); + + ASSERT_EQ(fd, uf.fd()); + + uf.reset(); + + ASSERT_EQ(-1, uf.fd()); +} + +TEST_F(FileTest, TestRestorableFileMoveConstruction) { + std::string testFile = GetTestFilePath(GetTestName()); + + RestorableFile rf1 = RestorableFile::CreateWritableFile(testFile, 0600); + int fd = rf1.fd(); + + RestorableFile rf2(std::move(rf1)); + + ASSERT_EQ(fd, rf2.fd()); + ASSERT_EQ(testFile, rf2.path()); +} + +TEST_F(FileTest, TestRestorableFileAssignment) { + std::string testFile1 = GetTestFilePath(GetTestName()); + std::string testFile2 = GetTestFilePath(GetTestName() + "2"); + + RestorableFile rf1 = RestorableFile::CreateWritableFile(testFile1, 0600); + int fd1 = rf1.fd(); + + RestorableFile rf2 = RestorableFile::CreateWritableFile(testFile2, 0600); + int fd2 = rf2.fd(); + + ASSERT_EQ(fd2, rf2.fd()); + ASSERT_EQ(testFile2, rf2.path()); + + rf2 = std::move(rf1); + + ASSERT_EQ(fd1, rf2.fd()); + ASSERT_EQ(testFile1, rf2.path()); +} + +TEST_F(FileTest, TestRestorableFileVerifyUniqueFileWithReset) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + + ASSERT_FILE_EXISTING(tmpFile); + + const UniqueFile& uf = rf.GetUniqueFile(); + + ASSERT_EQ(rf.fd(), uf.fd()); + ASSERT_EQ(rf.path(), uf.path()); + + rf.reset(); + + ASSERT_EQ(rf.fd(), uf.fd()); + ASSERT_EQ(rf.path(), uf.path()); + ASSERT_EQ(-1, rf.fd()); + ASSERT_TRUE(rf.path().empty()); + } +} + +TEST_F(FileTest, TestRestorableFileVerifyUniqueFileWithCommit) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + + ASSERT_FILE_EXISTING(tmpFile); + + const UniqueFile& uf = rf.GetUniqueFile(); + + ASSERT_EQ(rf.fd(), uf.fd()); + ASSERT_EQ(rf.path(), uf.path()); + + ASSERT_TRUE(rf.CreateBackupFile()); + + ASSERT_FILE_NOT_EXISTING(backupFile); + + rf.CommitWorkFile(); + + ASSERT_EQ(rf.fd(), uf.fd()); + ASSERT_EQ(rf.path(), uf.path()); + ASSERT_EQ(-1, rf.fd()); + ASSERT_EQ(testFile, rf.path()); + } +} + +TEST_F(FileTest, TestRestorableFileNewFileNotCommitted) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + + ASSERT_FILE_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(testFile); + + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + + ASSERT_FILE_CONTENT(tmpFile, "NewContent"); + } + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(testFile); +} + +TEST_F(FileTest, TestRestorableFileNotCommittedWithOriginal) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + + ASSERT_FILE_CONTENT(tmpFile, "NewContent"); + ASSERT_FILE_EXISTING(testFile); + } + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_CONTENT(testFile, "OriginalContent"); +} + +TEST_F(FileTest, TestRestorableFileNotCommittedWithOriginalAndOldTmp) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + CreateTestFileWithContents(testFile + kTmpFileSuffix, "OldTmp"); + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + + ASSERT_FILE_CONTENT(tmpFile, "NewContent"); + ASSERT_FILE_EXISTING(testFile); + } + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_CONTENT(testFile, "OriginalContent"); +} + +TEST_F(FileTest, TestRestorableFileNewFileCommitted) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + + ASSERT_FILE_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(testFile); + + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + ASSERT_FILE_CONTENT(tmpFile, "NewContent"); + + ASSERT_TRUE(rf.CreateBackupFile()); + + ASSERT_FILE_NOT_EXISTING(backupFile); + + ASSERT_TRUE(rf.CommitWorkFile()); + rf.RemoveBackupFile(); + + ASSERT_FILE_CONTENT(testFile, "NewContent"); + } + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(backupFile); + ASSERT_FILE_CONTENT(testFile, "NewContent"); +} + +TEST_F(FileTest, TestRestorableFileCommittedWithOriginal) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + ASSERT_FILE_CONTENT(tmpFile, "NewContent"); + + ASSERT_TRUE(rf.CreateBackupFile()); + + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_EXISTING(backupFile); + + ASSERT_TRUE(rf.CommitWorkFile()); + + ASSERT_FILE_EXISTING(backupFile); + ASSERT_FILE_CONTENT(testFile, "NewContent"); + + rf.RemoveBackupFile(); + + ASSERT_FILE_NOT_EXISTING(backupFile); + } + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_CONTENT(testFile, "NewContent"); +} + +TEST_F(FileTest, TestRestorableFileCommittedWithOriginalAndOldTmp) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + CreateTestFileWithContents(testFile + kTmpFileSuffix, "OldTmp"); + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + ASSERT_FILE_CONTENT(tmpFile, "NewContent"); + + ASSERT_TRUE(rf.CommitWorkFile()); + + ASSERT_FILE_CONTENT(testFile, "NewContent"); + } + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_CONTENT(testFile, "NewContent"); +} + +TEST_F(FileTest, TestRestorableFileCommitFailureNoOriginal) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + + ASSERT_TRUE(rf.CreateBackupFile()); + + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(backupFile); + + // Now remove tmp file to force commit failure. + close(rf.fd()); + ASSERT_EQ(0, unlink(tmpFile.c_str())); + ASSERT_FILE_NOT_EXISTING(tmpFile); + + ASSERT_FALSE(rf.CommitWorkFile()); + + ASSERT_EQ(-1, rf.fd()); + ASSERT_EQ(testFile, rf.path()); + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(tmpFile); + + ASSERT_TRUE(rf.RestoreBackupFile()); + } + + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(backupFile); +} + +TEST_F(FileTest, TestRestorableFileCommitFailureAndRollback) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + + ASSERT_TRUE(rf.CreateBackupFile()); + + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_EXISTING(backupFile); + + // Now remove tmp file to force commit failure. + close(rf.fd()); + ASSERT_EQ(0, unlink(tmpFile.c_str())); + ASSERT_FILE_NOT_EXISTING(tmpFile); + + ASSERT_FALSE(rf.CommitWorkFile()); + + ASSERT_EQ(-1, rf.fd()); + ASSERT_EQ(testFile, rf.path()); + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_EXISTING(backupFile); + + ASSERT_TRUE(rf.RestoreBackupFile()); + } + + ASSERT_FILE_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(backupFile); +} + +TEST_F(FileTest, TestRestorableFileResetAndRemoveAllFiles) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + + { + RestorableFile rf = RestorableFile::CreateWritableFile(testFile, 0600); + ASSERT_WRITE_TO_FD(rf.fd(), "NewContent"); + + ASSERT_TRUE(rf.CreateBackupFile()); + + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_EXISTING(backupFile); + + rf.ResetAndRemoveAllFiles(); + + ASSERT_EQ(-1, rf.fd()); + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(backupFile); + } + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(backupFile); +} + +TEST_F(FileTest, TestRestorableFileRemoveFileAndTmpFileWithContentFile) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + + RestorableFile::RemoveAllFiles(testFile); + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(backupFile); +} + +TEST_F(FileTest, TestRestorableFileRemoveFileAndTmpFileWithContentAndTmpFile) { + std::string testFile = GetTestFilePath(GetTestName()); + std::string tmpFile = testFile + kTmpFileSuffix; + std::string backupFile = testFile + kBackupFileSuffix; + CreateTestFileWithContents(testFile, "OriginalContent"); + CreateTestFileWithContents(testFile + kTmpFileSuffix, "TmpContent"); + + RestorableFile::RemoveAllFiles(testFile); + + ASSERT_FILE_NOT_EXISTING(tmpFile); + ASSERT_FILE_NOT_EXISTING(testFile); + ASSERT_FILE_NOT_EXISTING(backupFile); +} + +} // namespace installd +} // namespace android diff --git a/cmds/installd/tests/installd_file_test.xml b/cmds/installd/tests/installd_file_test.xml new file mode 100644 index 0000000000..5ec6e3f019 --- /dev/null +++ b/cmds/installd/tests/installd_file_test.xml @@ -0,0 +1,35 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2021 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<!-- Note: this is derived from the autogenerated configuration. We require + root support. --> +<configuration description="Runs installd_file_test."> + <option name="test-suite-tag" value="apct" /> + <option name="test-suite-tag" value="apct-native" /> + + <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer"> + <option name="cleanup" value="true" /> + <option name="push" + value="installd_file_test->/data/local/tmp/installd_file_test" /> + </target_preparer> + + <!-- The test requires root for file access (rollback. --> + <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer" /> + + <test class="com.android.tradefed.testtype.GTest" > + <option name="native-test-device-path" value="/data/local/tmp" /> + <option name="module-name" value="installd_file_test" /> + </test> +</configuration> diff --git a/cmds/servicemanager/servicemanager.rc b/cmds/servicemanager/servicemanager.rc index 0dd29e05ba..e5d689ff91 100644 --- a/cmds/servicemanager/servicemanager.rc +++ b/cmds/servicemanager/servicemanager.rc @@ -6,8 +6,8 @@ service servicemanager /system/bin/servicemanager onrestart restart apexd onrestart restart audioserver onrestart restart gatekeeperd - onrestart class_restart main - onrestart class_restart hal - onrestart class_restart early_hal + onrestart class_restart --only-enabled main + onrestart class_restart --only-enabled hal + onrestart class_restart --only-enabled early_hal task_profiles ServiceCapacityLow shutdown critical diff --git a/include/android/input.h b/include/android/input.h index bb98beb41a..76422154f1 100644 --- a/include/android/input.h +++ b/include/android/input.h @@ -947,9 +947,10 @@ int32_t AInputEvent_getSource(const AInputEvent* event); * and {@link AMotionEvent_fromJava()}. * After returning, the specified AInputEvent* object becomes invalid and should no longer be used. * The underlying Java object remains valid and does not change its state. + * + * Available since API level 31. */ - -void AInputEvent_release(const AInputEvent* event); +void AInputEvent_release(const AInputEvent* event) __INTRODUCED_IN(31); /*** Accessors for key events only. ***/ @@ -1001,8 +1002,10 @@ int64_t AKeyEvent_getEventTime(const AInputEvent* key_event); * Creates a native AInputEvent* object that is a copy of the specified Java android.view.KeyEvent. * The result may be used with generic and KeyEvent-specific AInputEvent_* functions. The object * returned by this function must be disposed using {@link AInputEvent_release()}. + * + * Available since API level 31. */ -const AInputEvent* AKeyEvent_fromJava(JNIEnv* env, jobject keyEvent); +const AInputEvent* AKeyEvent_fromJava(JNIEnv* env, jobject keyEvent) __INTRODUCED_IN(31); /*** Accessors for motion events only. ***/ @@ -1324,8 +1327,10 @@ float AMotionEvent_getHistoricalAxisValue(const AInputEvent* motion_event, * android.view.MotionEvent. The result may be used with generic and MotionEvent-specific * AInputEvent_* functions. The object returned by this function must be disposed using * {@link AInputEvent_release()}. + * + * Available since API level 31. */ -const AInputEvent* AMotionEvent_fromJava(JNIEnv* env, jobject motionEvent); +const AInputEvent* AMotionEvent_fromJava(JNIEnv* env, jobject motionEvent) __INTRODUCED_IN(31); struct AInputQueue; /** diff --git a/include/input/Input.h b/include/input/Input.h index 2e326cb102..dce6ccb3d5 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -1015,6 +1015,25 @@ private: std::queue<std::unique_ptr<DragEvent>> mDragEventPool; }; +/* + * Describes a unique request to enable or disable Pointer Capture. + */ +struct PointerCaptureRequest { +public: + inline PointerCaptureRequest() : enable(false), seq(0) {} + inline PointerCaptureRequest(bool enable, uint32_t seq) : enable(enable), seq(seq) {} + inline bool operator==(const PointerCaptureRequest& other) const { + return enable == other.enable && seq == other.seq; + } + explicit inline operator bool() const { return enable; } + + // True iff this is a request to enable Pointer Capture. + bool enable; + + // The sequence number for the request. + uint32_t seq; +}; + } // namespace android #endif // _LIBINPUT_INPUT_H diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 9e04ffeb11..55d3d70bd4 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -1410,23 +1410,6 @@ void IPCThreadState::threadDestructor(void *st) } } -status_t IPCThreadState::getProcessFreezeInfo(pid_t pid, bool *sync_received, bool *async_received) -{ - int ret = 0; - binder_frozen_status_info info; - info.pid = pid; - -#if defined(__ANDROID__) - if (ioctl(self()->mProcess->mDriverFD, BINDER_GET_FROZEN_INFO, &info) < 0) - ret = -errno; -#endif - *sync_received = info.sync_recv; - *async_received = info.async_recv; - - return ret; -} - -#ifndef __ANDROID_VNDK__ status_t IPCThreadState::getProcessFreezeInfo(pid_t pid, uint32_t *sync_received, uint32_t *async_received) { @@ -1443,7 +1426,6 @@ status_t IPCThreadState::getProcessFreezeInfo(pid_t pid, uint32_t *sync_received return ret; } -#endif status_t IPCThreadState::freeze(pid_t pid, bool enable, uint32_t timeout_ms) { struct binder_freeze_info info; diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h index 065e6e3b1d..82bebc95f4 100644 --- a/libs/binder/include/binder/IPCThreadState.h +++ b/libs/binder/include/binder/IPCThreadState.h @@ -51,17 +51,11 @@ public: static status_t freeze(pid_t pid, bool enabled, uint32_t timeout_ms); // Provide information about the state of a frozen process - static status_t getProcessFreezeInfo(pid_t pid, bool *sync_received, - bool *async_received); - - // TODO: Remove the above legacy duplicated function in next version -#ifndef __ANDROID_VNDK__ static status_t getProcessFreezeInfo(pid_t pid, uint32_t *sync_received, uint32_t *async_received); -#endif sp<ProcessState> process(); - + status_t clearLastError(); /** diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index 9670d7bba6..8dbdc1d115 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -16,6 +16,7 @@ #pragma once +#include <array> #include <map> // for legacy reasons #include <string> #include <type_traits> @@ -224,6 +225,15 @@ public: return writeData(val); } + template <typename T, size_t N> + status_t writeFixedArray(const std::array<T, N>& val) { + return writeData(val); + } + template <typename T, size_t N> + status_t writeFixedArray(const std::optional<std::array<T, N>>& val) { + return writeData(val); + } + // Write an Enum vector with underlying type int8_t. // Does not use padding; each byte is contiguous. template<typename T, std::enable_if_t<std::is_enum_v<T> && std::is_same_v<typename std::underlying_type_t<T>,int8_t>, bool> = 0> @@ -487,6 +497,15 @@ public: std::unique_ptr<std::vector<std::unique_ptr<std::string>>>* val) const __attribute__((deprecated("use std::optional version instead"))); status_t readUtf8VectorFromUtf16Vector(std::vector<std::string>* val) const; + template <typename T, size_t N> + status_t readFixedArray(std::array<T, N>* val) const { + return readData(val); + } + template <typename T, size_t N> + status_t readFixedArray(std::optional<std::array<T, N>>* val) const { + return readData(val); + } + template<typename T> status_t read(Flattenable<T>& val) const; @@ -818,6 +837,16 @@ private: || is_specialization_v<T, std::unique_ptr> || is_specialization_v<T, std::shared_ptr>; + // Tells if T is a fixed-size array. + template <typename T> + struct is_fixed_array : std::false_type {}; + + template <typename T, size_t N> + struct is_fixed_array<std::array<T, N>> : std::true_type {}; + + template <typename T> + static inline constexpr bool is_fixed_array_v = is_fixed_array<T>::value; + // special int32 value to indicate NonNull or Null parcelables // This is fixed to be only 0 or 1 by contract, do not change. static constexpr int32_t kNonNullParcelableFlag = 1; @@ -922,7 +951,9 @@ private: if (!c) return writeData(static_cast<int32_t>(kNullVectorSize)); } else if constexpr (std::is_base_of_v<Parcelable, T>) { if (!c) return writeData(static_cast<int32_t>(kNullParcelableFlag)); - } else /* constexpr */ { // could define this, but raise as error. + } else if constexpr (is_fixed_array_v<T>) { + if (!c) return writeData(static_cast<int32_t>(kNullVectorSize)); + } else /* constexpr */ { // could define this, but raise as error. static_assert(dependent_false_v<CT>); } return writeData(*c); @@ -961,6 +992,23 @@ private: return OK; } + template <typename T, size_t N> + status_t writeData(const std::array<T, N>& val) { + static_assert(N <= std::numeric_limits<int32_t>::max()); + status_t status = writeData(static_cast<int32_t>(N)); + if (status != OK) return status; + if constexpr (is_pointer_equivalent_array_v<T>) { + static_assert(N <= std::numeric_limits<size_t>::max() / sizeof(T)); + return write(val.data(), val.size() * sizeof(T)); + } else /* constexpr */ { + for (const auto& t : val) { + status = writeData(t); + if (status != OK) return status; + } + return OK; + } + } + // readData function overloads. // Implementation detail: Function overloading improves code readability over // template overloading, but prevents readData<T> from being used for those types. @@ -1053,9 +1101,8 @@ private: int32_t peek; status_t status = readData(&peek); if (status != OK) return status; - if constexpr (is_specialization_v<T, std::vector> - || std::is_same_v<T, String16> - || std::is_same_v<T, std::string>) { + if constexpr (is_specialization_v<T, std::vector> || is_fixed_array_v<T> || + std::is_same_v<T, String16> || std::is_same_v<T, std::string>) { if (peek == kNullVectorSize) { c->reset(); return OK; @@ -1065,12 +1112,15 @@ private: c->reset(); return OK; } - } else /* constexpr */ { // could define this, but raise as error. + } else /* constexpr */ { // could define this, but raise as error. static_assert(dependent_false_v<CT>); } // create a new object. if constexpr (is_specialization_v<CT, std::optional>) { - c->emplace(); + // Call default constructor explicitly + // - Clang bug: https://bugs.llvm.org/show_bug.cgi?id=35748 + // std::optional::emplace() doesn't work with nested types. + c->emplace(T()); } else /* constexpr */ { T* const t = new (std::nothrow) T; // contents read from Parcel below. if (t == nullptr) return NO_MEMORY; @@ -1079,7 +1129,7 @@ private: // rewind data ptr to reread (this is pretty quick), otherwise we could // pass an optional argument to readData to indicate a peeked value. setDataPosition(startPos); - if constexpr (is_specialization_v<T, std::vector>) { + if constexpr (is_specialization_v<T, std::vector> || is_fixed_array_v<T>) { return readData(&**c, READ_FLAG_SP_NULLABLE); // nullable sp<> allowed now } else { return readData(&**c); @@ -1142,6 +1192,41 @@ private: return OK; } + template <typename T, size_t N> + status_t readData(std::array<T, N>* val, ReadFlags readFlags = READ_FLAG_NONE) const { + static_assert(N <= std::numeric_limits<int32_t>::max()); + int32_t size; + status_t status = readInt32(&size); + if (status != OK) return status; + if (size < 0) return UNEXPECTED_NULL; + if (size != static_cast<int32_t>(N)) return BAD_VALUE; + if constexpr (is_pointer_equivalent_array_v<T>) { + auto data = reinterpret_cast<const T*>(readInplace(N * sizeof(T))); + if (data == nullptr) return BAD_VALUE; + memcpy(val->data(), data, N * sizeof(T)); + } else if constexpr (is_specialization_v<T, sp>) { + for (auto& t : *val) { + if (readFlags & READ_FLAG_SP_NULLABLE) { + status = readNullableStrongBinder(&t); // allow nullable + } else { + status = readStrongBinder(&t); + } + if (status != OK) return status; + } + } else if constexpr (is_fixed_array_v<T>) { // pass readFlags down to nested arrays + for (auto& t : *val) { + status = readData(&t, readFlags); + if (status != OK) return status; + } + } else /* constexpr */ { + for (auto& t : *val) { + status = readData(&t); + if (status != OK) return status; + } + } + return OK; + } + //----------------------------------------------------------------------------- private: diff --git a/libs/binder/ndk/Android.bp b/libs/binder/ndk/Android.bp index ee46fcb884..42895740ea 100644 --- a/libs/binder/ndk/Android.bp +++ b/libs/binder/ndk/Android.bp @@ -38,7 +38,7 @@ cc_defaults { host: { cflags: [ "-D__INTRODUCED_IN(n)=", - "-D__assert(a,b,c)=", + "-D__assert(a,b,c)=do { syslog(LOG_ERR, a \": \" c); abort(); } while(false)", // We want all the APIs to be available on the host. "-D__ANDROID_API__=10000", ], diff --git a/libs/binder/ndk/include_cpp/android/binder_parcel_utils.h b/libs/binder/ndk/include_cpp/android/binder_parcel_utils.h index 67623a6556..0bf1e3dba9 100644 --- a/libs/binder/ndk/include_cpp/android/binder_parcel_utils.h +++ b/libs/binder/ndk/include_cpp/android/binder_parcel_utils.h @@ -31,6 +31,7 @@ #include <android/binder_internal_logging.h> #include <android/binder_parcel.h> +#include <array> #include <optional> #include <string> #include <type_traits> @@ -86,9 +87,87 @@ static inline constexpr bool is_nullable_parcelable_v = is_parcelable_v<first_te (is_specialization_v<T, std::optional> || is_specialization_v<T, std::unique_ptr>); +// Tells if T is a fixed-size array. +template <typename T> +struct is_fixed_array : std::false_type {}; + +template <typename T, size_t N> +struct is_fixed_array<std::array<T, N>> : std::true_type {}; + +template <typename T> +static inline constexpr bool is_fixed_array_v = is_fixed_array<T>::value; + +template <typename T> +static inline constexpr bool dependent_false_v = false; } // namespace /** + * This checks the length against the array size and retrieves the buffer. No allocation required. + */ +template <typename T, size_t N> +static inline bool AParcel_stdArrayAllocator(void* arrayData, int32_t length, T** outBuffer) { + if (length < 0) return false; + + if (length != static_cast<int32_t>(N)) { + return false; + } + + std::array<T, N>* arr = static_cast<std::array<T, N>*>(arrayData); + *outBuffer = arr->data(); + return true; +} + +/** + * This checks the length against the array size and retrieves the buffer. No allocation required. + */ +template <typename T, size_t N> +static inline bool AParcel_nullableStdArrayAllocator(void* arrayData, int32_t length, + T** outBuffer) { + std::optional<std::array<T, N>>* arr = static_cast<std::optional<std::array<T, N>>*>(arrayData); + if (length < 0) { + *arr = std::nullopt; + return true; + } + + if (length != static_cast<int32_t>(N)) { + return false; + } + + arr->emplace(); + *outBuffer = (*arr)->data(); + return true; +} + +/** + * This checks the length against the array size. No allocation required. + */ +template <size_t N> +static inline bool AParcel_stdArrayExternalAllocator(void* arrayData, int32_t length) { + (void)arrayData; + return length == static_cast<int32_t>(N); +} + +/** + * This checks the length against the array size. No allocation required. + */ +template <typename T, size_t N> +static inline bool AParcel_nullableStdArrayExternalAllocator(void* arrayData, int32_t length) { + std::optional<std::array<T, N>>* arr = static_cast<std::optional<std::array<T, N>>*>(arrayData); + + if (length < 0) { + *arr = std::nullopt; + return true; + } + + if (length != static_cast<int32_t>(N)) { + return false; + } + + arr->emplace(); + return true; +} + +/** * This retrieves and allocates a vector to size 'length' and returns the underlying buffer. */ template <typename T> @@ -397,6 +476,118 @@ static inline const char* AParcel_nullableStdVectorStringElementGetter(const voi } /** + * This retrieves the underlying value in a std::array which may not be contiguous at index from a + * corresponding arrData. + */ +template <typename T, size_t N> +static inline T AParcel_stdArrayGetter(const void* arrData, size_t index) { + const std::array<T, N>* arr = static_cast<const std::array<T, N>*>(arrData); + return (*arr)[index]; +} + +/** + * This sets the underlying value in a corresponding arrData which may not be contiguous at + * index. + */ +template <typename T, size_t N> +static inline void AParcel_stdArraySetter(void* arrData, size_t index, T value) { + std::array<T, N>* arr = static_cast<std::array<T, N>*>(arrData); + (*arr)[index] = value; +} + +/** + * This retrieves the underlying value in a std::array which may not be contiguous at index from a + * corresponding arrData. + */ +template <typename T, size_t N> +static inline T AParcel_nullableStdArrayGetter(const void* arrData, size_t index) { + const std::optional<std::array<T, N>>* arr = + static_cast<const std::optional<std::array<T, N>>*>(arrData); + return (*arr)[index]; +} + +/** + * This sets the underlying value in a corresponding arrData which may not be contiguous at + * index. + */ +template <typename T, size_t N> +static inline void AParcel_nullableStdArraySetter(void* arrData, size_t index, T value) { + std::optional<std::array<T, N>>* arr = static_cast<std::optional<std::array<T, N>>*>(arrData); + (*arr)->at(index) = value; +} + +/** + * Allocates a std::string inside of std::array<std::string, N> at index 'index' to size 'length'. + */ +template <size_t N> +static inline bool AParcel_stdArrayStringElementAllocator(void* arrData, size_t index, + int32_t length, char** buffer) { + std::array<std::string, N>* arr = static_cast<std::array<std::string, N>*>(arrData); + std::string& element = arr->at(index); + return AParcel_stdStringAllocator(static_cast<void*>(&element), length, buffer); +} + +/** + * This gets the length and buffer of a std::string inside of a std::array<std::string, N> at index + * 'index'. + */ +template <size_t N> +static const char* AParcel_stdArrayStringElementGetter(const void* arrData, size_t index, + int32_t* outLength) { + const std::array<std::string, N>* arr = static_cast<const std::array<std::string, N>*>(arrData); + const std::string& element = arr->at(index); + + *outLength = static_cast<int32_t>(element.size()); + return element.c_str(); +} + +/** + * Allocates a std::string inside of std::array<std::optional<std::string>, N> at index 'index' to + * size 'length'. + */ +template <size_t N> +static inline bool AParcel_stdArrayNullableStringElementAllocator(void* arrData, size_t index, + int32_t length, char** buffer) { + std::array<std::optional<std::string>, N>* arr = + static_cast<std::array<std::optional<std::string>, N>*>(arrData); + std::optional<std::string>& element = arr->at(index); + return AParcel_nullableStdStringAllocator(static_cast<void*>(&element), length, buffer); +} + +/** + * This gets the length and buffer of a std::string inside of a + * std::array<std::optional<std::string>, N> at index 'index'. + */ +template <size_t N> +static const char* AParcel_stdArrayNullableStringElementGetter(const void* arrData, size_t index, + int32_t* outLength) { + const std::array<std::optional<std::string>, N>* arr = + static_cast<const std::array<std::optional<std::string>, N>*>(arrData); + const std::optional<std::string>& element = arr->at(index); + + if (!element) { + *outLength = -1; + return nullptr; + } + + *outLength = static_cast<int32_t>(element->size()); + return element->c_str(); +} + +/** + * Allocates a std::string inside of std::optional<std::array<std::optional<std::string>, N>> at + * index 'index' to size 'length'. + */ +template <size_t N> +static inline bool AParcel_nullableStdArrayStringElementAllocator(void* arrData, size_t index, + int32_t length, char** buffer) { + std::optional<std::array<std::optional<std::string>, N>>* arr = + static_cast<std::optional<std::array<std::optional<std::string>, N>>*>(arrData); + std::optional<std::string>& element = (*arr)->at(index); + return AParcel_nullableStdStringAllocator(static_cast<void*>(&element), length, buffer); +} + +/** * Convenience API for writing a std::string. */ static inline binder_status_t AParcel_writeString(AParcel* parcel, const std::string& str) { @@ -482,9 +673,7 @@ static inline binder_status_t AParcel_readVector( template <typename P> static inline binder_status_t AParcel_writeParcelable(AParcel* parcel, const P& p) { if constexpr (is_interface_v<P>) { - if (!p) { - return STATUS_UNEXPECTED_NULL; - } + // Legacy behavior: allow null return first_template_type_t<P>::writeToParcel(parcel, p); } else { static_assert(is_parcelable_v<P>); @@ -502,13 +691,8 @@ static inline binder_status_t AParcel_writeParcelable(AParcel* parcel, const P& template <typename P> static inline binder_status_t AParcel_readParcelable(const AParcel* parcel, P* p) { if constexpr (is_interface_v<P>) { - binder_status_t status = first_template_type_t<P>::readFromParcel(parcel, p); - if (status == STATUS_OK) { - if (!*p) { - return STATUS_UNEXPECTED_NULL; - } - } - return status; + // Legacy behavior: allow null + return first_template_type_t<P>::readFromParcel(parcel, p); } else { static_assert(is_parcelable_v<P>); int32_t null; @@ -560,7 +744,7 @@ static inline binder_status_t AParcel_readNullableParcelable(const AParcel* parc *p = std::nullopt; return STATUS_OK; } - *p = std::optional<first_template_type_t<P>>(first_template_type_t<P>{}); + p->emplace(first_template_type_t<P>()); return (*p)->readFromParcel(parcel); } else { static_assert(is_specialization_v<P, std::unique_ptr>); @@ -578,6 +762,64 @@ static inline binder_status_t AParcel_readNullableParcelable(const AParcel* parc } } +// Forward decls +template <typename T> +static inline binder_status_t AParcel_writeData(AParcel* parcel, const T& value); +template <typename T> +static inline binder_status_t AParcel_writeNullableData(AParcel* parcel, const T& value); +template <typename T> +static inline binder_status_t AParcel_readData(const AParcel* parcel, T* value); +template <typename T> +static inline binder_status_t AParcel_readNullableData(const AParcel* parcel, T* value); + +/** + * Reads an object of type T inside a std::array<T, N> at index 'index' from 'parcel'. + */ +template <typename T, size_t N> +binder_status_t AParcel_readStdArrayData(const AParcel* parcel, void* arrayData, size_t index) { + std::array<T, N>* arr = static_cast<std::array<T, N>*>(arrayData); + return AParcel_readData(parcel, &arr->at(index)); +} + +/** + * Reads a nullable object of type T inside a std::array<T, N> at index 'index' from 'parcel'. + */ +template <typename T, size_t N> +binder_status_t AParcel_readStdArrayNullableData(const AParcel* parcel, void* arrayData, + size_t index) { + std::array<T, N>* arr = static_cast<std::array<T, N>*>(arrayData); + return AParcel_readNullableData(parcel, &arr->at(index)); +} + +/** + * Reads a nullable object of type T inside a std::array<T, N> at index 'index' from 'parcel'. + */ +template <typename T, size_t N> +binder_status_t AParcel_readNullableStdArrayNullableData(const AParcel* parcel, void* arrayData, + size_t index) { + std::optional<std::array<T, N>>* arr = static_cast<std::optional<std::array<T, N>>*>(arrayData); + return AParcel_readNullableData(parcel, &(*arr)->at(index)); +} + +/** + * Writes an object of type T inside a std::array<T, N> at index 'index' to 'parcel'. + */ +template <typename T, size_t N> +binder_status_t AParcel_writeStdArrayData(AParcel* parcel, const void* arrayData, size_t index) { + const std::array<T, N>* arr = static_cast<const std::array<T, N>*>(arrayData); + return AParcel_writeData(parcel, arr->at(index)); +} + +/** + * Writes a nullable object of type T inside a std::array<T, N> at index 'index' to 'parcel'. + */ +template <typename T, size_t N> +binder_status_t AParcel_writeStdArrayNullableData(AParcel* parcel, const void* arrayData, + size_t index) { + const std::array<T, N>* arr = static_cast<const std::array<T, N>*>(arrayData); + return AParcel_writeNullableData(parcel, arr->at(index)); +} + /** * Writes a parcelable object of type P inside a std::vector<P> at index 'index' to 'parcel'. */ @@ -721,9 +963,25 @@ inline binder_status_t AParcel_readNullableStdVectorParcelableElement<SpAIBinder */ template <typename P> static inline binder_status_t AParcel_writeVector(AParcel* parcel, const std::vector<P>& vec) { - const void* vectorData = static_cast<const void*>(&vec); - return AParcel_writeParcelableArray(parcel, vectorData, static_cast<int32_t>(vec.size()), - AParcel_writeStdVectorParcelableElement<P>); + if constexpr (std::is_enum_v<P>) { + if constexpr (std::is_same_v<std::underlying_type_t<P>, int8_t>) { + return AParcel_writeByteArray(parcel, reinterpret_cast<const int8_t*>(vec.data()), + static_cast<int32_t>(vec.size())); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int32_t>) { + return AParcel_writeInt32Array(parcel, reinterpret_cast<const int32_t*>(vec.data()), + static_cast<int32_t>(vec.size())); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int64_t>) { + return AParcel_writeInt64Array(parcel, reinterpret_cast<const int64_t*>(vec.data()), + static_cast<int32_t>(vec.size())); + } else { + static_assert(dependent_false_v<P>, "unrecognized type"); + } + } else { + static_assert(!std::is_same_v<P, std::string>, "specialization should be used"); + const void* vectorData = static_cast<const void*>(&vec); + return AParcel_writeParcelableArray(parcel, vectorData, static_cast<int32_t>(vec.size()), + AParcel_writeStdVectorParcelableElement<P>); + } } /** @@ -731,9 +989,24 @@ static inline binder_status_t AParcel_writeVector(AParcel* parcel, const std::ve */ template <typename P> static inline binder_status_t AParcel_readVector(const AParcel* parcel, std::vector<P>* vec) { - void* vectorData = static_cast<void*>(vec); - return AParcel_readParcelableArray(parcel, vectorData, AParcel_stdVectorExternalAllocator<P>, - AParcel_readStdVectorParcelableElement<P>); + if constexpr (std::is_enum_v<P>) { + void* vectorData = static_cast<void*>(vec); + if constexpr (std::is_same_v<std::underlying_type_t<P>, int8_t>) { + return AParcel_readByteArray(parcel, vectorData, AParcel_stdVectorAllocator<int8_t>); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int32_t>) { + return AParcel_readInt32Array(parcel, vectorData, AParcel_stdVectorAllocator<int32_t>); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int64_t>) { + return AParcel_readInt64Array(parcel, vectorData, AParcel_stdVectorAllocator<int64_t>); + } else { + static_assert(dependent_false_v<P>, "unrecognized type"); + } + } else { + static_assert(!std::is_same_v<P, std::string>, "specialization should be used"); + void* vectorData = static_cast<void*>(vec); + return AParcel_readParcelableArray(parcel, vectorData, + AParcel_stdVectorExternalAllocator<P>, + AParcel_readStdVectorParcelableElement<P>); + } } /** @@ -742,10 +1015,30 @@ static inline binder_status_t AParcel_readVector(const AParcel* parcel, std::vec template <typename P> static inline binder_status_t AParcel_writeVector(AParcel* parcel, const std::optional<std::vector<P>>& vec) { - if (!vec) return AParcel_writeInt32(parcel, -1); - const void* vectorData = static_cast<const void*>(&vec); - return AParcel_writeParcelableArray(parcel, vectorData, static_cast<int32_t>(vec->size()), - AParcel_writeNullableStdVectorParcelableElement<P>); + if constexpr (std::is_enum_v<P>) { + if constexpr (std::is_same_v<std::underlying_type_t<P>, int8_t>) { + return AParcel_writeByteArray( + parcel, vec ? reinterpret_cast<const int8_t*>(vec->data()) : nullptr, + vec ? static_cast<int32_t>(vec->size()) : -1); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int32_t>) { + return AParcel_writeInt32Array( + parcel, vec ? reinterpret_cast<const int32_t*>(vec->data()) : nullptr, + vec ? static_cast<int32_t>(vec->size()) : -1); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int64_t>) { + return AParcel_writeInt64Array( + parcel, vec ? reinterpret_cast<const int64_t*>(vec->data()) : nullptr, + vec ? static_cast<int32_t>(vec->size()) : -1); + } else { + static_assert(dependent_false_v<P>, "unrecognized type"); + } + } else { + static_assert(!std::is_same_v<P, std::optional<std::string>>, + "specialization should be used"); + if (!vec) return AParcel_writeInt32(parcel, -1); + const void* vectorData = static_cast<const void*>(&vec); + return AParcel_writeParcelableArray(parcel, vectorData, static_cast<int32_t>(vec->size()), + AParcel_writeNullableStdVectorParcelableElement<P>); + } } /** @@ -754,10 +1047,28 @@ static inline binder_status_t AParcel_writeVector(AParcel* parcel, template <typename P> static inline binder_status_t AParcel_readVector(const AParcel* parcel, std::optional<std::vector<P>>* vec) { - void* vectorData = static_cast<void*>(vec); - return AParcel_readParcelableArray(parcel, vectorData, - AParcel_nullableStdVectorExternalAllocator<P>, - AParcel_readNullableStdVectorParcelableElement<P>); + if constexpr (std::is_enum_v<P>) { + void* vectorData = static_cast<void*>(vec); + if constexpr (std::is_same_v<std::underlying_type_t<P>, int8_t>) { + return AParcel_readByteArray(parcel, vectorData, + AParcel_nullableStdVectorAllocator<int8_t>); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int32_t>) { + return AParcel_readInt32Array(parcel, vectorData, + AParcel_nullableStdVectorAllocator<int32_t>); + } else if constexpr (std::is_same_v<std::underlying_type_t<P>, int64_t>) { + return AParcel_readInt64Array(parcel, vectorData, + AParcel_nullableStdVectorAllocator<int64_t>); + } else { + static_assert(dependent_false_v<P>, "unrecognized type"); + } + } else { + static_assert(!std::is_same_v<P, std::optional<std::string>>, + "specialization should be used"); + void* vectorData = static_cast<void*>(vec); + return AParcel_readParcelableArray(parcel, vectorData, + AParcel_nullableStdVectorExternalAllocator<P>, + AParcel_readNullableStdVectorParcelableElement<P>); + } } // @START @@ -1139,6 +1450,294 @@ static inline binder_status_t AParcel_resizeVector(const AParcel* parcel, return STATUS_OK; } +/** + * Writes a fixed-size array of T. + */ +template <typename T, size_t N> +static inline binder_status_t AParcel_writeFixedArray(AParcel* parcel, + const std::array<T, N>& arr) { + if constexpr (std::is_same_v<T, bool>) { + const void* arrayData = static_cast<const void*>(&arr); + return AParcel_writeBoolArray(parcel, arrayData, static_cast<int32_t>(N), + &AParcel_stdArrayGetter<T, N>); + } else if constexpr (std::is_same_v<T, uint8_t>) { + return AParcel_writeByteArray(parcel, reinterpret_cast<const int8_t*>(arr.data()), + static_cast<int32_t>(arr.size())); + } else if constexpr (std::is_same_v<T, char16_t>) { + return AParcel_writeCharArray(parcel, arr.data(), static_cast<int32_t>(arr.size())); + } else if constexpr (std::is_same_v<T, int32_t>) { + return AParcel_writeInt32Array(parcel, arr.data(), static_cast<int32_t>(arr.size())); + } else if constexpr (std::is_same_v<T, int64_t>) { + return AParcel_writeInt64Array(parcel, arr.data(), static_cast<int32_t>(arr.size())); + } else if constexpr (std::is_same_v<T, float>) { + return AParcel_writeFloatArray(parcel, arr.data(), static_cast<int32_t>(arr.size())); + } else if constexpr (std::is_same_v<T, double>) { + return AParcel_writeDoubleArray(parcel, arr.data(), static_cast<int32_t>(arr.size())); + } else if constexpr (std::is_same_v<T, std::string>) { + const void* arrayData = static_cast<const void*>(&arr); + return AParcel_writeStringArray(parcel, arrayData, static_cast<int32_t>(N), + &AParcel_stdArrayStringElementGetter<N>); + } else { + const void* arrayData = static_cast<const void*>(&arr); + return AParcel_writeParcelableArray(parcel, arrayData, static_cast<int32_t>(N), + &AParcel_writeStdArrayData<T, N>); + } +} + +/** + * Writes a fixed-size array of T. + */ +template <typename T, size_t N> +static inline binder_status_t AParcel_writeFixedArrayWithNullableData(AParcel* parcel, + const std::array<T, N>& arr) { + if constexpr (std::is_same_v<T, bool> || std::is_same_v<T, uint8_t> || + std::is_same_v<T, char16_t> || std::is_same_v<T, int32_t> || + std::is_same_v<T, int64_t> || std::is_same_v<T, float> || + std::is_same_v<T, double> || std::is_same_v<T, std::string>) { + return AParcel_writeFixedArray(parcel, arr); + } else if constexpr (std::is_same_v<T, std::optional<std::string>>) { + const void* arrayData = static_cast<const void*>(&arr); + return AParcel_writeStringArray(parcel, arrayData, static_cast<int32_t>(N), + &AParcel_stdArrayNullableStringElementGetter<N>); + } else { + const void* arrayData = static_cast<const void*>(&arr); + return AParcel_writeParcelableArray(parcel, arrayData, static_cast<int32_t>(N), + &AParcel_writeStdArrayNullableData<T, N>); + } +} + +/** + * Writes a fixed-size array of T. + */ +template <typename T, size_t N> +static inline binder_status_t AParcel_writeNullableFixedArrayWithNullableData( + AParcel* parcel, const std::optional<std::array<T, N>>& arr) { + if (!arr) return AParcel_writeInt32(parcel, -1); + return AParcel_writeFixedArrayWithNullableData(parcel, arr.value()); +} + +/** + * Reads a fixed-size array of T. + */ +template <typename T, size_t N> +static inline binder_status_t AParcel_readFixedArray(const AParcel* parcel, std::array<T, N>* arr) { + void* arrayData = static_cast<void*>(arr); + if constexpr (std::is_same_v<T, bool>) { + return AParcel_readBoolArray(parcel, arrayData, &AParcel_stdArrayExternalAllocator<N>, + &AParcel_stdArraySetter<T, N>); + } else if constexpr (std::is_same_v<T, uint8_t>) { + return AParcel_readByteArray(parcel, arrayData, &AParcel_stdArrayAllocator<int8_t, N>); + } else if constexpr (std::is_same_v<T, char16_t>) { + return AParcel_readCharArray(parcel, arrayData, &AParcel_stdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, int32_t>) { + return AParcel_readInt32Array(parcel, arrayData, &AParcel_stdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, int64_t>) { + return AParcel_readInt64Array(parcel, arrayData, &AParcel_stdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, float>) { + return AParcel_readFloatArray(parcel, arrayData, &AParcel_stdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, double>) { + return AParcel_readDoubleArray(parcel, arrayData, &AParcel_stdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, std::string>) { + return AParcel_readStringArray(parcel, arrayData, &AParcel_stdArrayExternalAllocator<N>, + &AParcel_stdArrayStringElementAllocator<N>); + } else { + return AParcel_readParcelableArray(parcel, arrayData, &AParcel_stdArrayExternalAllocator<N>, + &AParcel_readStdArrayData<T, N>); + } +} + +/** + * Reads a fixed-size array of T. + */ +template <typename T, size_t N> +static inline binder_status_t AParcel_readFixedArrayWithNullableData(const AParcel* parcel, + std::array<T, N>* arr) { + void* arrayData = static_cast<void*>(arr); + if constexpr (std::is_same_v<T, bool> || std::is_same_v<T, uint8_t> || + std::is_same_v<T, char16_t> || std::is_same_v<T, int32_t> || + std::is_same_v<T, int64_t> || std::is_same_v<T, float> || + std::is_same_v<T, double> || std::is_same_v<T, std::string>) { + return AParcel_readFixedArray(parcel, arr); + } else if constexpr (std::is_same_v<T, std::optional<std::string>>) { + return AParcel_readStringArray(parcel, arrayData, &AParcel_stdArrayExternalAllocator<N>, + &AParcel_stdArrayNullableStringElementAllocator<N>); + } else { + return AParcel_readParcelableArray(parcel, arrayData, &AParcel_stdArrayExternalAllocator<N>, + &AParcel_readStdArrayNullableData<T, N>); + } +} + +/** + * Reads a fixed-size array of T. + */ +template <typename T, size_t N> +static inline binder_status_t AParcel_readNullableFixedArrayWithNullableData( + const AParcel* parcel, std::optional<std::array<T, N>>* arr) { + void* arrayData = static_cast<void*>(arr); + if constexpr (std::is_same_v<T, bool>) { + return AParcel_readBoolArray(parcel, arrayData, + &AParcel_nullableStdArrayExternalAllocator<T, N>, + &AParcel_nullableStdArraySetter<T, N>); + } else if constexpr (std::is_same_v<T, uint8_t>) { + return AParcel_readByteArray(parcel, arrayData, + &AParcel_nullableStdArrayAllocator<int8_t, N>); + } else if constexpr (std::is_same_v<T, char16_t>) { + return AParcel_readCharArray(parcel, arrayData, &AParcel_nullableStdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, int32_t>) { + return AParcel_readInt32Array(parcel, arrayData, &AParcel_nullableStdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, int64_t>) { + return AParcel_readInt64Array(parcel, arrayData, &AParcel_nullableStdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, float>) { + return AParcel_readFloatArray(parcel, arrayData, &AParcel_nullableStdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, double>) { + return AParcel_readDoubleArray(parcel, arrayData, &AParcel_nullableStdArrayAllocator<T, N>); + } else if constexpr (std::is_same_v<T, std::string>) { + return AParcel_readStringArray(parcel, arrayData, + &AParcel_nullableStdArrayExternalAllocator<N>, + &AParcel_nullableStdArrayStringElementAllocator<N>); + } else { + return AParcel_readParcelableArray(parcel, arrayData, + &AParcel_nullableStdArrayExternalAllocator<T, N>, + &AParcel_readStdArrayNullableData<T, N>); + } +} + +/** + * Convenience API for writing a value of any type. + */ +template <typename T> +static inline binder_status_t AParcel_writeData(AParcel* parcel, const T& value) { + if constexpr (is_specialization_v<T, std::vector>) { + return AParcel_writeVector(parcel, value); + } else if constexpr (is_fixed_array_v<T>) { + return AParcel_writeFixedArray(parcel, value); + } else if constexpr (std::is_same_v<std::string, T>) { + return AParcel_writeString(parcel, value); + } else if constexpr (std::is_same_v<bool, T>) { + return AParcel_writeBool(parcel, value); + } else if constexpr (std::is_same_v<int8_t, T> || std::is_same_v<uint8_t, T>) { + return AParcel_writeByte(parcel, value); + } else if constexpr (std::is_same_v<char16_t, T>) { + return AParcel_writeChar(parcel, value); + } else if constexpr (std::is_same_v<int32_t, T>) { + return AParcel_writeInt32(parcel, value); + } else if constexpr (std::is_same_v<int64_t, T>) { + return AParcel_writeInt64(parcel, value); + } else if constexpr (std::is_same_v<float, T>) { + return AParcel_writeFloat(parcel, value); + } else if constexpr (std::is_same_v<double, T>) { + return AParcel_writeDouble(parcel, value); + } else if constexpr (std::is_same_v<ScopedFileDescriptor, T>) { + return AParcel_writeRequiredParcelFileDescriptor(parcel, value); + } else if constexpr (std::is_same_v<SpAIBinder, T>) { + return AParcel_writeRequiredStrongBinder(parcel, value); + } else if constexpr (std::is_enum_v<T>) { + return AParcel_writeData(parcel, static_cast<std::underlying_type_t<T>>(value)); + } else if constexpr (is_interface_v<T>) { + return AParcel_writeParcelable(parcel, value); + } else if constexpr (is_parcelable_v<T>) { + return AParcel_writeParcelable(parcel, value); + } else { + static_assert(dependent_false_v<T>, "unrecognized type"); + return STATUS_OK; + } +} + +/** + * Convenience API for writing a nullable value of any type. + */ +template <typename T> +static inline binder_status_t AParcel_writeNullableData(AParcel* parcel, const T& value) { + if constexpr (is_specialization_v<T, std::optional> && + is_specialization_v<first_template_type_t<T>, std::vector>) { + return AParcel_writeVector(parcel, value); + } else if constexpr (is_specialization_v<T, std::optional> && + is_fixed_array_v<first_template_type_t<T>>) { + return AParcel_writeNullableFixedArrayWithNullableData(parcel, value); + } else if constexpr (is_fixed_array_v<T>) { // happens with a nullable multi-dimensional array. + return AParcel_writeFixedArrayWithNullableData(parcel, value); + } else if constexpr (is_specialization_v<T, std::optional> && + std::is_same_v<first_template_type_t<T>, std::string>) { + return AParcel_writeString(parcel, value); + } else if constexpr (is_nullable_parcelable_v<T> || is_interface_v<T>) { + return AParcel_writeNullableParcelable(parcel, value); + } else if constexpr (std::is_same_v<ScopedFileDescriptor, T>) { + return AParcel_writeNullableParcelFileDescriptor(parcel, value); + } else if constexpr (std::is_same_v<SpAIBinder, T>) { + return AParcel_writeNullableStrongBinder(parcel, value); + } else { + return AParcel_writeData(parcel, value); + } +} + +/** + * Convenience API for reading a value of any type. + */ +template <typename T> +static inline binder_status_t AParcel_readData(const AParcel* parcel, T* value) { + if constexpr (is_specialization_v<T, std::vector>) { + return AParcel_readVector(parcel, value); + } else if constexpr (is_fixed_array_v<T>) { + return AParcel_readFixedArray(parcel, value); + } else if constexpr (std::is_same_v<std::string, T>) { + return AParcel_readString(parcel, value); + } else if constexpr (std::is_same_v<bool, T>) { + return AParcel_readBool(parcel, value); + } else if constexpr (std::is_same_v<int8_t, T> || std::is_same_v<uint8_t, T>) { + return AParcel_readByte(parcel, value); + } else if constexpr (std::is_same_v<char16_t, T>) { + return AParcel_readChar(parcel, value); + } else if constexpr (std::is_same_v<int32_t, T>) { + return AParcel_readInt32(parcel, value); + } else if constexpr (std::is_same_v<int64_t, T>) { + return AParcel_readInt64(parcel, value); + } else if constexpr (std::is_same_v<float, T>) { + return AParcel_readFloat(parcel, value); + } else if constexpr (std::is_same_v<double, T>) { + return AParcel_readDouble(parcel, value); + } else if constexpr (std::is_same_v<ScopedFileDescriptor, T>) { + return AParcel_readRequiredParcelFileDescriptor(parcel, value); + } else if constexpr (std::is_same_v<SpAIBinder, T>) { + return AParcel_readRequiredStrongBinder(parcel, value); + } else if constexpr (std::is_enum_v<T>) { + return AParcel_readData(parcel, reinterpret_cast<std::underlying_type_t<T>*>(value)); + } else if constexpr (is_interface_v<T>) { + return AParcel_readParcelable(parcel, value); + } else if constexpr (is_parcelable_v<T>) { + return AParcel_readParcelable(parcel, value); + } else { + static_assert(dependent_false_v<T>, "unrecognized type"); + return STATUS_OK; + } +} + +/** + * Convenience API for reading a nullable value of any type. + */ +template <typename T> +static inline binder_status_t AParcel_readNullableData(const AParcel* parcel, T* value) { + if constexpr (is_specialization_v<T, std::optional> && + is_specialization_v<first_template_type_t<T>, std::vector>) { + return AParcel_readVector(parcel, value); + } else if constexpr (is_specialization_v<T, std::optional> && + is_fixed_array_v<first_template_type_t<T>>) { + return AParcel_readNullableFixedArrayWithNullableData(parcel, value); + } else if constexpr (is_fixed_array_v<T>) { // happens with a nullable multi-dimensional array. + return AParcel_readFixedArrayWithNullableData(parcel, value); + } else if constexpr (is_specialization_v<T, std::optional> && + std::is_same_v<first_template_type_t<T>, std::string>) { + return AParcel_readString(parcel, value); + } else if constexpr (is_nullable_parcelable_v<T> || is_interface_v<T>) { + return AParcel_readNullableParcelable(parcel, value); + } else if constexpr (std::is_same_v<ScopedFileDescriptor, T>) { + return AParcel_readNullableParcelFileDescriptor(parcel, value); + } else if constexpr (std::is_same_v<SpAIBinder, T>) { + return AParcel_readNullableStrongBinder(parcel, value); + } else { + return AParcel_readData(parcel, value); + } +} + } // namespace ndk /** @} */ diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 3d2eddf611..4d6b294000 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -66,6 +66,35 @@ pub trait Interface: Send + Sync { } } +/// Implemented by sync interfaces to specify what the associated async interface is. +/// Generic to handle the fact that async interfaces are generic over a thread pool. +/// +/// The binder in any object implementing this trait should be compatible with the +/// `Target` associated type, and using `FromIBinder` to convert it to the target +/// should not fail. +pub trait ToAsyncInterface<P> +where + Self: Interface, + Self::Target: FromIBinder, +{ + /// The async interface associated with this sync interface. + type Target: ?Sized; +} + +/// Implemented by async interfaces to specify what the associated sync interface is. +/// +/// The binder in any object implementing this trait should be compatible with the +/// `Target` associated type, and using `FromIBinder` to convert it to the target +/// should not fail. +pub trait ToSyncInterface +where + Self: Interface, + Self::Target: FromIBinder, +{ + /// The sync interface associated with this async interface. + type Target: ?Sized; +} + /// Interface stability promise /// /// An interface can promise to be a stable vendor interface ([`Vintf`]), or @@ -337,6 +366,26 @@ impl<I: FromIBinder + ?Sized> Strong<I> { pub fn downgrade(this: &Strong<I>) -> Weak<I> { Weak::new(this) } + + /// Convert this synchronous binder handle into an asynchronous one. + pub fn into_async<P>(self) -> Strong<<I as ToAsyncInterface<P>>::Target> + where + I: ToAsyncInterface<P>, + { + // By implementing the ToAsyncInterface trait, it is guaranteed that the binder + // object is also valid for the target type. + FromIBinder::try_from(self.0.as_binder()).unwrap() + } + + /// Convert this asynchronous binder handle into a synchronous one. + pub fn into_sync(self) -> Strong<<I as ToSyncInterface>::Target> + where + I: ToSyncInterface, + { + // By implementing the ToSyncInterface trait, it is guaranteed that the binder + // object is also valid for the target type. + FromIBinder::try_from(self.0.as_binder()).unwrap() + } } impl<I: FromIBinder + ?Sized> Clone for Strong<I> { @@ -1017,6 +1066,14 @@ macro_rules! declare_binder_interface { .expect(concat!("Error cloning interface ", stringify!($async_interface))) } } + + impl<P: $crate::BinderAsyncPool> $crate::ToAsyncInterface<P> for dyn $interface { + type Target = dyn $async_interface<P>; + } + + impl<P: $crate::BinderAsyncPool> $crate::ToSyncInterface for dyn $async_interface<P> { + type Target = dyn $interface; + } )? }; } diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index b94dfa137e..7c04a7207b 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -109,8 +109,8 @@ pub mod parcel; pub use crate::binder::{ BinderFeatures, FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, - Stability, Strong, TransactionCode, TransactionFlags, Weak, FIRST_CALL_TRANSACTION, - FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, LAST_CALL_TRANSACTION, + Stability, Strong, ToAsyncInterface, ToSyncInterface, TransactionCode, TransactionFlags, Weak, + FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, LAST_CALL_TRANSACTION, }; pub use crate::binder_async::{BoxFuture, BinderAsyncPool}; pub use error::{status_t, ExceptionCode, Result, Status, StatusCode}; diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index 40359b4749..80dc47682c 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -100,6 +100,7 @@ enum TestTransactionCode { Test = FIRST_CALL_TRANSACTION, GetDumpArgs, GetSelinuxContext, + GetIsHandlingTransaction, } impl TryFrom<u32> for TestTransactionCode { @@ -112,6 +113,7 @@ impl TryFrom<u32> for TestTransactionCode { _ if c == TestTransactionCode::GetSelinuxContext as u32 => { Ok(TestTransactionCode::GetSelinuxContext) } + _ if c == TestTransactionCode::GetIsHandlingTransaction as u32 => Ok(TestTransactionCode::GetIsHandlingTransaction), _ => Err(StatusCode::UNKNOWN_TRANSACTION), } } @@ -140,6 +142,10 @@ impl ITest for TestService { ThreadState::with_calling_sid(|sid| sid.map(|s| s.to_string_lossy().into_owned())); sid.ok_or(StatusCode::UNEXPECTED_NULL) } + + fn get_is_handling_transaction(&self) -> binder::Result<bool> { + Ok(binder::is_handling_transaction()) + } } /// Trivial testing binder interface @@ -152,6 +158,9 @@ pub trait ITest: Interface { /// Returns the caller's SELinux context fn get_selinux_context(&self) -> binder::Result<String>; + + /// Returns the value of calling `is_handling_transaction`. + fn get_is_handling_transaction(&self) -> binder::Result<bool>; } /// Async trivial testing binder interface @@ -164,6 +173,9 @@ pub trait IATest<P>: Interface { /// Returns the caller's SELinux context fn get_selinux_context(&self) -> binder::BoxFuture<'static, binder::Result<String>>; + + /// Returns the value of calling `is_handling_transaction`. + fn get_is_handling_transaction(&self) -> binder::BoxFuture<'static, binder::Result<bool>>; } declare_binder_interface! { @@ -186,6 +198,7 @@ fn on_transact( TestTransactionCode::Test => reply.write(&service.test()?), TestTransactionCode::GetDumpArgs => reply.write(&service.get_dump_args()?), TestTransactionCode::GetSelinuxContext => reply.write(&service.get_selinux_context()?), + TestTransactionCode::GetIsHandlingTransaction => reply.write(&service.get_is_handling_transaction()?), } } @@ -212,6 +225,15 @@ impl ITest for BpTest { )?; reply.read() } + + fn get_is_handling_transaction(&self) -> binder::Result<bool> { + let reply = self.binder.transact( + TestTransactionCode::GetIsHandlingTransaction as TransactionCode, + 0, + |_| Ok(()), + )?; + reply.read() + } } impl<P: binder::BinderAsyncPool> IATest<P> for BpTest { @@ -238,6 +260,14 @@ impl<P: binder::BinderAsyncPool> IATest<P> for BpTest { |reply| async move { reply?.read() } ) } + + fn get_is_handling_transaction(&self) -> binder::BoxFuture<'static, binder::Result<bool>> { + let binder = self.binder.clone(); + P::spawn( + move || binder.transact(TestTransactionCode::GetIsHandlingTransaction as TransactionCode, 0, |_| Ok(())), + |reply| async move { reply?.read() } + ) + } } impl ITest for Binder<BnTest> { @@ -252,6 +282,10 @@ impl ITest for Binder<BnTest> { fn get_selinux_context(&self) -> binder::Result<String> { self.0.get_selinux_context() } + + fn get_is_handling_transaction(&self) -> binder::Result<bool> { + self.0.get_is_handling_transaction() + } } impl<P: binder::BinderAsyncPool> IATest<P> for Binder<BnTest> { @@ -269,6 +303,11 @@ impl<P: binder::BinderAsyncPool> IATest<P> for Binder<BnTest> { let res = self.0.get_selinux_context(); Box::pin(async move { res }) } + + fn get_is_handling_transaction(&self) -> binder::BoxFuture<'static, binder::Result<bool>> { + let res = self.0.get_is_handling_transaction(); + Box::pin(async move { res }) + } } /// Trivial testing binder interface @@ -500,7 +539,7 @@ mod tests { #[tokio::test] async fn get_selinux_context_async() { - let service_name = "get_selinux_context"; + let service_name = "get_selinux_context_async"; let _process = ScopedServiceProcess::new(service_name); let test_client: Strong<dyn IATest<Tokio>> = binder_tokio::get_interface(service_name).await.expect("Did not get manager binder service"); @@ -510,6 +549,32 @@ mod tests { ); } + #[tokio::test] + async fn get_selinux_context_sync_to_async() { + let service_name = "get_selinux_context"; + let _process = ScopedServiceProcess::new(service_name); + let test_client: Strong<dyn ITest> = + binder::get_interface(service_name).expect("Did not get manager binder service"); + let test_client = test_client.into_async::<Tokio>(); + assert_eq!( + test_client.get_selinux_context().await.unwrap(), + get_expected_selinux_context() + ); + } + + #[tokio::test] + async fn get_selinux_context_async_to_sync() { + let service_name = "get_selinux_context"; + let _process = ScopedServiceProcess::new(service_name); + let test_client: Strong<dyn IATest<Tokio>> = + binder_tokio::get_interface(service_name).await.expect("Did not get manager binder service"); + let test_client = test_client.into_sync(); + assert_eq!( + test_client.get_selinux_context().unwrap(), + get_expected_selinux_context() + ); + } + struct Bools { binder_died: Arc<AtomicBool>, binder_dealloc: Arc<AtomicBool>, @@ -867,4 +932,45 @@ mod tests { Err(err) => assert_eq!(err, binder::StatusCode::BAD_VALUE), } } + + #[test] + fn get_is_handling_transaction() { + let service_name = "get_is_handling_transaction"; + let _process = ScopedServiceProcess::new(service_name); + let test_client: Strong<dyn ITest> = + binder::get_interface(service_name).expect("Did not get manager binder service"); + // Should be true externally. + assert!(test_client.get_is_handling_transaction().unwrap()); + + // Should be false locally. + assert!(!binder::is_handling_transaction()); + + // Should also be false in spawned thread. + std::thread::spawn(|| { + assert!(!binder::is_handling_transaction()); + }).join().unwrap(); + } + + #[tokio::test] + async fn get_is_handling_transaction_async() { + let service_name = "get_is_handling_transaction_async"; + let _process = ScopedServiceProcess::new(service_name); + let test_client: Strong<dyn IATest<Tokio>> = + binder_tokio::get_interface(service_name).await.expect("Did not get manager binder service"); + // Should be true externally. + assert!(test_client.get_is_handling_transaction().await.unwrap()); + + // Should be false locally. + assert!(!binder::is_handling_transaction()); + + // Should also be false in spawned task. + tokio::spawn(async { + assert!(!binder::is_handling_transaction()); + }).await.unwrap(); + + // And in spawn_blocking task. + tokio::task::spawn_blocking(|| { + assert!(!binder::is_handling_transaction()); + }).await.unwrap(); + } } diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 4d316f72f0..c8938998a6 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -496,7 +496,7 @@ TEST_F(BinderLibTest, Freeze) { EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, true, 1000)); EXPECT_EQ(FAILED_TRANSACTION, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply)); - bool sync_received, async_received; + uint32_t sync_received, async_received; EXPECT_EQ(NO_ERROR, IPCThreadState::self()->getProcessFreezeInfo(pid, &sync_received, &async_received)); @@ -504,15 +504,7 @@ TEST_F(BinderLibTest, Freeze) { EXPECT_EQ(sync_received, 1); EXPECT_EQ(async_received, 0); - uint32_t sync_received2, async_received2; - - EXPECT_EQ(NO_ERROR, IPCThreadState::self()->getProcessFreezeInfo(pid, &sync_received2, - &async_received2)); - - EXPECT_EQ(sync_received2, 1); - EXPECT_EQ(async_received2, 0); - - EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, 0, 0)); + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, false, 0)); EXPECT_EQ(NO_ERROR, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply)); } diff --git a/libs/binder/tests/parcel_fuzzer/binder.cpp b/libs/binder/tests/parcel_fuzzer/binder.cpp index 32406e599c..077d915ca1 100644 --- a/libs/binder/tests/parcel_fuzzer/binder.cpp +++ b/libs/binder/tests/parcel_fuzzer/binder.cpp @@ -233,6 +233,32 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { PARCEL_READ_WITH_STATUS(std::optional<std::vector<std::optional<std::string>>>, readUtf8VectorFromUtf16Vector), PARCEL_READ_WITH_STATUS(std::vector<std::string>, readUtf8VectorFromUtf16Vector), +#define COMMA , + PARCEL_READ_WITH_STATUS(std::array<uint8_t COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<uint8_t COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<char16_t COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<char16_t COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<std::string COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<std::optional<std::string> COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<android::String16 COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<std::optional<android::String16> COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<android::sp<android::IBinder> COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<android::sp<android::IBinder> COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<ExampleParcelable COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<std::optional<ExampleParcelable> COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<ByteEnum COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<ByteEnum COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<IntEnum COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<IntEnum COMMA 3>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<LongEnum COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<LongEnum COMMA 3>>, readFixedArray), + // nested arrays + PARCEL_READ_WITH_STATUS(std::array<std::array<uint8_t COMMA 3> COMMA 4>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<std::array<uint8_t COMMA 3> COMMA 4>>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::array<ExampleParcelable COMMA 3>, readFixedArray), + PARCEL_READ_WITH_STATUS(std::optional<std::array<std::array<std::optional<ExampleParcelable> COMMA 3> COMMA 4>>, readFixedArray), +#undef COMMA + [] (const android::Parcel& p, uint8_t /*len*/) { FUZZ_LOG() << "about to read flattenable"; ExampleFlattenable f; diff --git a/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp b/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp index 752fcbb34c..5aeb5cc6f2 100644 --- a/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp +++ b/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp @@ -156,5 +156,21 @@ std::vector<ParcelRead<NdkParcelAdapter>> BINDER_NDK_PARCEL_READ_FUNCTIONS{ PARCEL_READ(std::optional<std::vector<char16_t>>, ndk::AParcel_readVector), PARCEL_READ(std::vector<int32_t>, ndk::AParcel_resizeVector), PARCEL_READ(std::optional<std::vector<int32_t>>, ndk::AParcel_resizeVector), + + // methods for std::array<T,N> +#define COMMA , + PARCEL_READ(std::array<bool COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<uint8_t COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<char16_t COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<int32_t COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<int64_t COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<float COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<double COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<std::string COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<SomeParcelable COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<ndk::SpAIBinder COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<ndk::ScopedFileDescriptor COMMA 3>, ndk::AParcel_readData), + PARCEL_READ(std::array<std::shared_ptr<ISomeInterface> COMMA 3>, ndk::AParcel_readData), +#undef COMMA }; // clang-format on diff --git a/libs/graphicsenv/GraphicsEnv.cpp b/libs/graphicsenv/GraphicsEnv.cpp index d54de4999c..7f0cac5d4f 100644 --- a/libs/graphicsenv/GraphicsEnv.cpp +++ b/libs/graphicsenv/GraphicsEnv.cpp @@ -343,80 +343,6 @@ void* GraphicsEnv::loadLibrary(std::string name) { return nullptr; } -bool GraphicsEnv::checkAngleRules(void* so) { - auto manufacturer = base::GetProperty("ro.product.manufacturer", "UNSET"); - auto model = base::GetProperty("ro.product.model", "UNSET"); - - auto ANGLEGetFeatureSupportUtilAPIVersion = - (fpANGLEGetFeatureSupportUtilAPIVersion)dlsym(so, - "ANGLEGetFeatureSupportUtilAPIVersion"); - - if (!ANGLEGetFeatureSupportUtilAPIVersion) { - ALOGW("Cannot find ANGLEGetFeatureSupportUtilAPIVersion function"); - return false; - } - - // Negotiate the interface version by requesting most recent known to the platform - unsigned int versionToUse = CURRENT_ANGLE_API_VERSION; - if (!(ANGLEGetFeatureSupportUtilAPIVersion)(&versionToUse)) { - ALOGW("Cannot use ANGLE feature-support library, it is older than supported by EGL, " - "requested version %u", - versionToUse); - return false; - } - - // Add and remove versions below as needed - bool useAngle = false; - switch (versionToUse) { - case 2: { - ALOGV("Using version %d of ANGLE feature-support library", versionToUse); - void* rulesHandle = nullptr; - int rulesVersion = 0; - void* systemInfoHandle = nullptr; - - // Get the symbols for the feature-support-utility library: -#define GET_SYMBOL(symbol) \ - fp##symbol symbol = (fp##symbol)dlsym(so, #symbol); \ - if (!symbol) { \ - ALOGW("Cannot find " #symbol " in ANGLE feature-support library"); \ - break; \ - } - GET_SYMBOL(ANGLEAndroidParseRulesString); - GET_SYMBOL(ANGLEGetSystemInfo); - GET_SYMBOL(ANGLEAddDeviceInfoToSystemInfo); - GET_SYMBOL(ANGLEShouldBeUsedForApplication); - GET_SYMBOL(ANGLEFreeRulesHandle); - GET_SYMBOL(ANGLEFreeSystemInfoHandle); - - // Parse the rules, obtain the SystemInfo, and evaluate the - // application against the rules: - if (!(ANGLEAndroidParseRulesString)(mRulesBuffer.data(), &rulesHandle, &rulesVersion)) { - ALOGW("ANGLE feature-support library cannot parse rules file"); - break; - } - if (!(ANGLEGetSystemInfo)(&systemInfoHandle)) { - ALOGW("ANGLE feature-support library cannot obtain SystemInfo"); - break; - } - if (!(ANGLEAddDeviceInfoToSystemInfo)(manufacturer.c_str(), model.c_str(), - systemInfoHandle)) { - ALOGW("ANGLE feature-support library cannot add device info to SystemInfo"); - break; - } - useAngle = (ANGLEShouldBeUsedForApplication)(rulesHandle, rulesVersion, - systemInfoHandle, mAngleAppName.c_str()); - (ANGLEFreeRulesHandle)(rulesHandle); - (ANGLEFreeSystemInfoHandle)(systemInfoHandle); - } break; - - default: - ALOGW("Version %u of ANGLE feature-support library is NOT supported.", versionToUse); - } - - ALOGV("Close temporarily-loaded ANGLE opt-in/out logic"); - return useAngle; -} - bool GraphicsEnv::shouldUseAngle(std::string appName) { if (appName != mAngleAppName) { // Make sure we are checking the app we were init'ed for @@ -444,31 +370,20 @@ void GraphicsEnv::updateUseAngle() { const char* ANGLE_PREFER_ANGLE = "angle"; const char* ANGLE_PREFER_NATIVE = "native"; + mUseAngle = NO; if (mAngleDeveloperOptIn == ANGLE_PREFER_ANGLE) { ALOGV("User set \"Developer Options\" to force the use of ANGLE"); mUseAngle = YES; } else if (mAngleDeveloperOptIn == ANGLE_PREFER_NATIVE) { ALOGV("User set \"Developer Options\" to force the use of Native"); - mUseAngle = NO; } else { - // The "Developer Options" value wasn't set to force the use of ANGLE. Need to temporarily - // load ANGLE and call the updatable opt-in/out logic: - void* featureSo = loadLibrary("feature_support"); - if (featureSo) { - ALOGV("loaded ANGLE's opt-in/out logic from namespace"); - mUseAngle = checkAngleRules(featureSo) ? YES : NO; - dlclose(featureSo); - featureSo = nullptr; - } else { - ALOGV("Could not load the ANGLE opt-in/out logic, cannot use ANGLE."); - } + ALOGV("User set invalid \"Developer Options\": '%s'", mAngleDeveloperOptIn.c_str()); } } void GraphicsEnv::setAngleInfo(const std::string path, const std::string appName, const std::string developerOptIn, - const std::vector<std::string> eglFeatures, const int rulesFd, - const long rulesOffset, const long rulesLength) { + const std::vector<std::string> eglFeatures) { if (mUseAngle != UNKNOWN) { // We've already figured out an answer for this app, so just return. ALOGV("Already evaluated the rules file for '%s': use ANGLE = %s", appName.c_str(), @@ -485,22 +400,6 @@ void GraphicsEnv::setAngleInfo(const std::string path, const std::string appName ALOGV("setting ANGLE application opt-in to '%s'", developerOptIn.c_str()); mAngleDeveloperOptIn = developerOptIn; - lseek(rulesFd, rulesOffset, SEEK_SET); - mRulesBuffer = std::vector<char>(rulesLength + 1); - ssize_t numBytesRead = read(rulesFd, mRulesBuffer.data(), rulesLength); - if (numBytesRead < 0) { - ALOGE("Cannot read rules file: numBytesRead = %zd", numBytesRead); - numBytesRead = 0; - } else if (numBytesRead == 0) { - ALOGW("Empty rules file"); - } - if (numBytesRead != rulesLength) { - ALOGW("Did not read all of the necessary bytes from the rules file." - "expected: %ld, got: %zd", - rulesLength, numBytesRead); - } - mRulesBuffer[numBytesRead] = '\0'; - // Update the current status of whether we should use ANGLE or not updateUseAngle(); } diff --git a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h index 900fc49b59..56d1139f57 100644 --- a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h +++ b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h @@ -97,8 +97,7 @@ public: // in the search path must have a '!' after the zip filename, e.g. // /system/app/ANGLEPrebuilt/ANGLEPrebuilt.apk!/lib/arm64-v8a void setAngleInfo(const std::string path, const std::string appName, std::string devOptIn, - const std::vector<std::string> eglFeatures, const int rulesFd, - const long rulesOffset, const long rulesLength); + const std::vector<std::string> eglFeatures); // Get the ANGLE driver namespace. android_namespace_t* getAngleNamespace(); // Get the app name for ANGLE debug message. @@ -129,8 +128,6 @@ private: // Load requested ANGLE library. void* loadLibrary(std::string name); - // Check ANGLE support with the rules. - bool checkAngleRules(void* so); // Update whether ANGLE should be used. void updateUseAngle(); // Link updatable driver namespace with llndk and vndk-sp libs. @@ -159,8 +156,6 @@ private: std::string mAngleDeveloperOptIn; // ANGLE EGL features; std::vector<std::string> mAngleEglFeatures; - // ANGLE rules. - std::vector<char> mRulesBuffer; // Use ANGLE flag. UseAngle mUseAngle = UNKNOWN; // Vulkan debug layers libs. diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 56a9683773..94e1ae1c74 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -118,12 +118,12 @@ void BLASTBufferItemConsumer::getConnectionEvents(uint64_t frameNumber, bool* ne } void BLASTBufferItemConsumer::setBlastBufferQueue(BLASTBufferQueue* blastbufferqueue) { - Mutex::Autolock lock(mMutex); + std::scoped_lock lock(mBufferQueueMutex); mBLASTBufferQueue = blastbufferqueue; } void BLASTBufferItemConsumer::onSidebandStreamChanged() { - Mutex::Autolock lock(mMutex); + std::scoped_lock lock(mBufferQueueMutex); if (mBLASTBufferQueue != nullptr) { sp<NativeHandle> stream = getSidebandStream(); mBLASTBufferQueue->setSidebandStream(stream); @@ -630,7 +630,10 @@ bool BLASTBufferQueue::maxBuffersAcquired(bool includeExtraAcquire) const { class BBQSurface : public Surface { private: + std::mutex mMutex; sp<BLASTBufferQueue> mBbq; + bool mDestroyed = false; + public: BBQSurface(const sp<IGraphicBufferProducer>& igbp, bool controlledByApp, const sp<IBinder>& scHandle, const sp<BLASTBufferQueue>& bbq) @@ -650,6 +653,10 @@ public: status_t setFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override { + std::unique_lock _lock{mMutex}; + if (mDestroyed) { + return DEAD_OBJECT; + } if (!ValidateFrameRate(frameRate, compatibility, changeFrameRateStrategy, "BBQSurface::setFrameRate")) { return BAD_VALUE; @@ -658,8 +665,20 @@ public: } status_t setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInfo) override { + std::unique_lock _lock{mMutex}; + if (mDestroyed) { + return DEAD_OBJECT; + } return mBbq->setFrameTimelineInfo(frameTimelineInfo); } + + void destroy() override { + Surface::destroy(); + + std::unique_lock _lock{mMutex}; + mDestroyed = true; + mBbq = nullptr; + } }; // TODO: Can we coalesce this with frame updates? Need to confirm diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 2edb4e4ba4..353a91d062 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -2622,4 +2622,14 @@ status_t Surface::setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInf return composerService()->setFrameTimelineInfo(mGraphicBufferProducer, frameTimelineInfo); } +sp<IBinder> Surface::getSurfaceControlHandle() const { + Mutex::Autolock lock(mMutex); + return mSurfaceControlHandle; +} + +void Surface::destroy() { + Mutex::Autolock lock(mMutex); + mSurfaceControlHandle = nullptr; +} + }; // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index ea9b1c68af..6c5b2aa53c 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -62,11 +62,12 @@ private: uint64_t mCurrentFrameNumber = 0; Mutex mMutex; + std::mutex mBufferQueueMutex; ConsumerFrameEventHistory mFrameEventHistory GUARDED_BY(mMutex); std::queue<uint64_t> mDisconnectEvents GUARDED_BY(mMutex); bool mCurrentlyConnected GUARDED_BY(mMutex); bool mPreviouslyConnected GUARDED_BY(mMutex); - BLASTBufferQueue* mBLASTBufferQueue GUARDED_BY(mMutex); + BLASTBufferQueue* mBLASTBufferQueue GUARDED_BY(mBufferQueueMutex); }; class BLASTBufferQueue diff --git a/libs/gui/include/gui/Surface.h b/libs/gui/include/gui/Surface.h index 7e4143b1f3..e5403512a9 100644 --- a/libs/gui/include/gui/Surface.h +++ b/libs/gui/include/gui/Surface.h @@ -99,7 +99,7 @@ public: */ sp<IGraphicBufferProducer> getIGraphicBufferProducer() const; - sp<IBinder> getSurfaceControlHandle() const { return mSurfaceControlHandle; } + sp<IBinder> getSurfaceControlHandle() const; /* convenience function to check that the given surface is non NULL as * well as its IGraphicBufferProducer */ @@ -333,6 +333,7 @@ public: virtual int connect( int api, bool reportBufferRemoval, const sp<SurfaceListener>& sListener); + virtual void destroy(); // When client connects to Surface with reportBufferRemoval set to true, any buffers removed // from this Surface will be collected and returned here. Once this method returns, these diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp index 3c59f11395..94023e6247 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp @@ -424,14 +424,28 @@ base::unique_fd SkiaGLRenderEngine::flush() { return fenceFd; } -bool SkiaGLRenderEngine::waitFence(base::unique_fd fenceFd) { +void SkiaGLRenderEngine::waitFence(base::borrowed_fd fenceFd) { + if (fenceFd.get() >= 0 && !waitGpuFence(fenceFd)) { + ATRACE_NAME("SkiaGLRenderEngine::waitFence"); + sync_wait(fenceFd.get(), -1); + } +} + +bool SkiaGLRenderEngine::waitGpuFence(base::borrowed_fd fenceFd) { if (!gl::GLExtensions::getInstance().hasNativeFenceSync() || !gl::GLExtensions::getInstance().hasWaitSync()) { return false; } + // Duplicate the fence for passing to eglCreateSyncKHR. + base::unique_fd fenceDup(dup(fenceFd.get())); + if (fenceDup.get() < 0) { + ALOGE("failed to create duplicate fence fd: %d", fenceDup.get()); + return false; + } + // release the fd and transfer the ownership to EGLSync - EGLint attribs[] = {EGL_SYNC_NATIVE_FENCE_FD_ANDROID, fenceFd.release(), EGL_NONE}; + EGLint attribs[] = {EGL_SYNC_NATIVE_FENCE_FD_ANDROID, fenceDup.release(), EGL_NONE}; EGLSyncKHR sync = eglCreateSyncKHR(mEGLDisplay, EGL_SYNC_NATIVE_FENCE_ANDROID, attribs); if (sync == EGL_NO_SYNC_KHR) { ALOGE("failed to create EGL native fence sync: %#x", eglGetError()); @@ -726,14 +740,6 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, return NO_ERROR; } - if (bufferFence.get() >= 0) { - // Duplicate the fence for passing to waitFence. - base::unique_fd bufferFenceDup(dup(bufferFence.get())); - if (bufferFenceDup < 0 || !waitFence(std::move(bufferFenceDup))) { - ATRACE_NAME("Waiting before draw"); - sync_wait(bufferFence.get(), -1); - } - } if (buffer == nullptr) { ALOGE("No output buffer provided. Aborting GPU composition."); return BAD_VALUE; @@ -758,6 +764,9 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, true, mTextureCleanupMgr); } + // wait on the buffer to be ready to use prior to using it + waitFence(bufferFence); + const ui::Dataspace dstDataspace = mUseColorManagement ? display.outputDataspace : ui::Dataspace::V0_SRGB_LINEAR; sk_sp<SkSurface> dstSurface = surfaceTextureRef->getOrCreateSurface(dstDataspace, grContext); @@ -1014,6 +1023,12 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, false, mTextureCleanupMgr); } + // if the layer's buffer has a fence, then we must must respect the fence prior to using + // the buffer. + if (layer->source.buffer.fence != nullptr) { + waitFence(layer->source.buffer.fence->get()); + } + // isOpaque means we need to ignore the alpha in the image, // replacing it with the alpha specified by the LayerSettings. See // https://developer.android.com/reference/android/view/SurfaceControl.Builder#setOpaque(boolean) diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.h b/libs/renderengine/skia/SkiaGLRenderEngine.h index a852bbcaf8..238ad8f452 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.h +++ b/libs/renderengine/skia/SkiaGLRenderEngine.h @@ -99,7 +99,10 @@ private: inline GrDirectContext* getActiveGrContext() const; base::unique_fd flush(); - bool waitFence(base::unique_fd fenceFd); + // waitFence attempts to wait in the GPU, and if unable to waits on the CPU instead. + void waitFence(base::borrowed_fd fenceFd); + bool waitGpuFence(base::borrowed_fd fenceFd); + void initCanvas(SkCanvas* canvas, const DisplaySettings& display); void drawShadow(SkCanvas* canvas, const SkRRect& casterRRect, const ShadowSettings& shadowSettings); diff --git a/services/gpuservice/gpuservice.rc b/services/gpuservice/gpuservice.rc index 65a5c2776a..0da8bd3812 100644 --- a/services/gpuservice/gpuservice.rc +++ b/services/gpuservice/gpuservice.rc @@ -1,4 +1,4 @@ service gpu /system/bin/gpuservice class core user gpu_service - group graphics + group graphics readtracefs diff --git a/services/inputflinger/InputListener.cpp b/services/inputflinger/InputListener.cpp index 33b3e1ee2f..71b0f5fec9 100644 --- a/services/inputflinger/InputListener.cpp +++ b/services/inputflinger/InputListener.cpp @@ -287,16 +287,16 @@ void NotifyDeviceResetArgs::notify(const sp<InputListenerInterface>& listener) c // --- NotifyPointerCaptureChangedArgs --- -NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, - bool enabled) - : NotifyArgs(id, eventTime), enabled(enabled) {} +NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs( + int32_t id, nsecs_t eventTime, const PointerCaptureRequest& request) + : NotifyArgs(id, eventTime), request(request) {} NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs( const NotifyPointerCaptureChangedArgs& other) - : NotifyArgs(other.id, other.eventTime), enabled(other.enabled) {} + : NotifyArgs(other.id, other.eventTime), request(other.request) {} bool NotifyPointerCaptureChangedArgs::operator==(const NotifyPointerCaptureChangedArgs& rhs) const { - return id == rhs.id && eventTime == rhs.eventTime && enabled == rhs.enabled; + return id == rhs.id && eventTime == rhs.eventTime && request == rhs.request; } void NotifyPointerCaptureChangedArgs::notify(const sp<InputListenerInterface>& listener) const { diff --git a/services/inputflinger/InputReaderBase.cpp b/services/inputflinger/InputReaderBase.cpp index 9cc777d450..d34482f506 100644 --- a/services/inputflinger/InputReaderBase.cpp +++ b/services/inputflinger/InputReaderBase.cpp @@ -67,6 +67,9 @@ std::string InputReaderConfiguration::changesToString(uint32_t changes) { if (changes & CHANGE_EXTERNAL_STYLUS_PRESENCE) { result += "EXTERNAL_STYLUS_PRESENCE | "; } + if (changes & CHANGE_POINTER_CAPTURE) { + result += "POINTER_CAPTURE | "; + } if (changes & CHANGE_ENABLED_STATE) { result += "ENABLED_STATE | "; } diff --git a/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp b/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp index bc77b8aef4..aa8cc302c7 100644 --- a/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp +++ b/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp @@ -113,7 +113,7 @@ private: void onPointerDownOutsideFocus(const sp<IBinder>& newToken) override {} - void setPointerCapture(bool enabled) override {} + void setPointerCapture(const PointerCaptureRequest&) override {} void notifyDropWindow(const sp<IBinder>&, float x, float y) override {} diff --git a/services/inputflinger/dispatcher/Entry.cpp b/services/inputflinger/dispatcher/Entry.cpp index 881024fc6e..5c3747e2ec 100644 --- a/services/inputflinger/dispatcher/Entry.cpp +++ b/services/inputflinger/dispatcher/Entry.cpp @@ -119,15 +119,15 @@ std::string FocusEntry::getDescription() const { // PointerCaptureChanged notifications always go to apps, so set the flag POLICY_FLAG_PASS_TO_USER // for all entries. PointerCaptureChangedEntry::PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, - bool hasPointerCapture) + const PointerCaptureRequest& request) : EventEntry(id, Type::POINTER_CAPTURE_CHANGED, eventTime, POLICY_FLAG_PASS_TO_USER), - pointerCaptureEnabled(hasPointerCapture) {} + pointerCaptureRequest(request) {} PointerCaptureChangedEntry::~PointerCaptureChangedEntry() {} std::string PointerCaptureChangedEntry::getDescription() const { return StringPrintf("PointerCaptureChangedEvent(pointerCaptureEnabled=%s)", - pointerCaptureEnabled ? "true" : "false"); + pointerCaptureRequest.enable ? "true" : "false"); } // --- DragEntry --- @@ -324,8 +324,7 @@ CommandEntry::CommandEntry(Command command) keyEntry(nullptr), userActivityEventType(0), seq(0), - handled(false), - enabled(false) {} + handled(false) {} CommandEntry::~CommandEntry() {} diff --git a/services/inputflinger/dispatcher/Entry.h b/services/inputflinger/dispatcher/Entry.h index ebbd8e93bf..6f1dfadf59 100644 --- a/services/inputflinger/dispatcher/Entry.h +++ b/services/inputflinger/dispatcher/Entry.h @@ -104,9 +104,9 @@ struct FocusEntry : EventEntry { }; struct PointerCaptureChangedEntry : EventEntry { - bool pointerCaptureEnabled; + const PointerCaptureRequest pointerCaptureRequest; - PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, bool hasPointerCapture); + PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, const PointerCaptureRequest&); std::string getDescription() const override; ~PointerCaptureChangedEntry() override; @@ -284,7 +284,7 @@ struct CommandEntry { sp<IBinder> oldToken; sp<IBinder> newToken; std::string obscuringPackage; - bool enabled; + PointerCaptureRequest pointerCaptureRequest; int32_t pid; nsecs_t consumeTime; // time when the event was consumed by InputConsumer int32_t displayId; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index c2a2794eb9..6e9430a975 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -523,7 +523,6 @@ InputDispatcher::InputDispatcher(const sp<InputDispatcherPolicyInterface>& polic mInTouchMode(true), mMaximumObscuringOpacityForTouch(1.0f), mFocusedDisplayId(ADISPLAY_ID_DEFAULT), - mFocusedWindowRequestedPointerCapture(false), mWindowTokenWithPointerCapture(nullptr), mLatencyAggregator(), mLatencyTracker(&mLatencyAggregator), @@ -1311,36 +1310,51 @@ void InputDispatcher::dispatchFocusLocked(nsecs_t currentTime, std::shared_ptr<F void InputDispatcher::dispatchPointerCaptureChangedLocked( nsecs_t currentTime, const std::shared_ptr<PointerCaptureChangedEntry>& entry, DropReason& dropReason) { - const bool haveWindowWithPointerCapture = mWindowTokenWithPointerCapture != nullptr; - if (entry->pointerCaptureEnabled && haveWindowWithPointerCapture) { - LOG_ALWAYS_FATAL("Pointer Capture has already been enabled for the window."); - } - if (!entry->pointerCaptureEnabled && !haveWindowWithPointerCapture) { - // Pointer capture was already forcefully disabled because of focus change. - dropReason = DropReason::NOT_DROPPED; - return; - } - - // Set drop reason for early returns - dropReason = DropReason::NO_POINTER_CAPTURE; + dropReason = DropReason::NOT_DROPPED; + const bool haveWindowWithPointerCapture = mWindowTokenWithPointerCapture != nullptr; sp<IBinder> token; - if (entry->pointerCaptureEnabled) { - // Enable Pointer Capture - if (!mFocusedWindowRequestedPointerCapture) { + + if (entry->pointerCaptureRequest.enable) { + // Enable Pointer Capture. + if (haveWindowWithPointerCapture && + (entry->pointerCaptureRequest == mCurrentPointerCaptureRequest)) { + LOG_ALWAYS_FATAL("This request to enable Pointer Capture has already been dispatched " + "to the window."); + } + if (!mCurrentPointerCaptureRequest.enable) { // This can happen if a window requests capture and immediately releases capture. ALOGW("No window requested Pointer Capture."); + dropReason = DropReason::NO_POINTER_CAPTURE; + return; + } + if (entry->pointerCaptureRequest.seq != mCurrentPointerCaptureRequest.seq) { + ALOGI("Skipping dispatch of Pointer Capture being enabled: sequence number mismatch."); return; } + token = mFocusResolver.getFocusedWindowToken(mFocusedDisplayId); LOG_ALWAYS_FATAL_IF(!token, "Cannot find focused window for Pointer Capture."); mWindowTokenWithPointerCapture = token; } else { - // Disable Pointer Capture + // Disable Pointer Capture. + // We do not check if the sequence number matches for requests to disable Pointer Capture + // for two reasons: + // 1. Pointer Capture can be disabled by a focus change, which means we can get two entries + // to disable capture with the same sequence number: one generated by + // disablePointerCaptureForcedLocked() and another as an acknowledgement of Pointer + // Capture being disabled in InputReader. + // 2. We respect any request to disable Pointer Capture generated by InputReader, since the + // actual Pointer Capture state that affects events being generated by input devices is + // in InputReader. + if (!haveWindowWithPointerCapture) { + // Pointer capture was already forcefully disabled because of focus change. + dropReason = DropReason::NOT_DROPPED; + return; + } token = mWindowTokenWithPointerCapture; mWindowTokenWithPointerCapture = nullptr; - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } } @@ -1349,8 +1363,7 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( if (channel == nullptr) { // Window has gone away, clean up Pointer Capture state. mWindowTokenWithPointerCapture = nullptr; - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } return; @@ -3185,7 +3198,7 @@ void InputDispatcher::startDispatchCycleLocked(nsecs_t currentTime, static_cast<const PointerCaptureChangedEntry&>(eventEntry); status = connection->inputPublisher .publishCaptureEvent(dispatchEntry->seq, captureEntry.id, - captureEntry.pointerCaptureEnabled); + captureEntry.pointerCaptureRequest.enable); break; } @@ -3990,14 +4003,14 @@ void InputDispatcher::notifyDeviceReset(const NotifyDeviceResetArgs* args) { void InputDispatcher::notifyPointerCaptureChanged(const NotifyPointerCaptureChangedArgs* args) { #if DEBUG_INBOUND_EVENT_DETAILS ALOGD("notifyPointerCaptureChanged - eventTime=%" PRId64 ", enabled=%s", args->eventTime, - args->enabled ? "true" : "false"); + args->request.enable ? "true" : "false"); #endif bool needWake; { // acquire lock std::scoped_lock _l(mLock); auto entry = std::make_unique<PointerCaptureChangedEntry>(args->id, args->eventTime, - args->enabled); + args->request); needWake = enqueueInboundEventLocked(std::move(entry)); } // release lock @@ -4941,8 +4954,8 @@ void InputDispatcher::logDispatchStateLocked() { std::string InputDispatcher::dumpPointerCaptureStateLocked() { std::string dump; - dump += StringPrintf(INDENT "FocusedWindowRequestedPointerCapture: %s\n", - toString(mFocusedWindowRequestedPointerCapture)); + dump += StringPrintf(INDENT "Pointer Capture Requested: %s\n", + toString(mCurrentPointerCaptureRequest.enable)); std::string windowName = "None"; if (mWindowTokenWithPointerCapture) { @@ -4951,7 +4964,7 @@ std::string InputDispatcher::dumpPointerCaptureStateLocked() { windowName = captureWindowHandle ? captureWindowHandle->getName().c_str() : "token has capture without window"; } - dump += StringPrintf(INDENT "CurrentWindowWithPointerCapture: %s\n", windowName.c_str()); + dump += StringPrintf(INDENT "Current Window with Pointer Capture: %s\n", windowName.c_str()); return dump; } @@ -5419,14 +5432,13 @@ void InputDispatcher::requestPointerCapture(const sp<IBinder>& windowToken, bool return; } - if (enabled == mFocusedWindowRequestedPointerCapture) { + if (enabled == mCurrentPointerCaptureRequest.enable) { ALOGW("Ignoring request to %s Pointer Capture: " "window has %s requested pointer capture.", enabled ? "enable" : "disable", enabled ? "already" : "not"); return; } - mFocusedWindowRequestedPointerCapture = enabled; setPointerCaptureLocked(enabled); } // release lock @@ -6185,14 +6197,13 @@ void InputDispatcher::onFocusChangedLocked(const FocusResolver::FocusChanges& ch } void InputDispatcher::disablePointerCaptureForcedLocked() { - if (!mFocusedWindowRequestedPointerCapture && !mWindowTokenWithPointerCapture) { + if (!mCurrentPointerCaptureRequest.enable && !mWindowTokenWithPointerCapture) { return; } ALOGD_IF(DEBUG_FOCUS, "Disabling Pointer Capture because the window lost focus."); - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } @@ -6209,14 +6220,16 @@ void InputDispatcher::disablePointerCaptureForcedLocked() { } auto entry = std::make_unique<PointerCaptureChangedEntry>(mIdGenerator.nextId(), now(), - false /* hasCapture */); + mCurrentPointerCaptureRequest); mInboundQueue.push_front(std::move(entry)); } void InputDispatcher::setPointerCaptureLocked(bool enabled) { + mCurrentPointerCaptureRequest.enable = enabled; + mCurrentPointerCaptureRequest.seq++; std::unique_ptr<CommandEntry> commandEntry = std::make_unique<CommandEntry>( &InputDispatcher::doSetPointerCaptureLockedInterruptible); - commandEntry->enabled = enabled; + commandEntry->pointerCaptureRequest = mCurrentPointerCaptureRequest; postCommandLocked(std::move(commandEntry)); } @@ -6224,7 +6237,7 @@ void InputDispatcher::doSetPointerCaptureLockedInterruptible( android::inputdispatcher::CommandEntry* commandEntry) { mLock.unlock(); - mPolicy->setPointerCapture(commandEntry->enabled); + mPolicy->setPointerCapture(commandEntry->pointerCaptureRequest); mLock.lock(); } diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index 9edf41c9c0..30652c65ee 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -357,10 +357,12 @@ private: // Keeps track of the focused window per display and determines focus changes. FocusResolver mFocusResolver GUARDED_BY(mLock); - // Whether the focused window on the focused display has requested Pointer Capture. - // The state of this variable should always be in sync with the state of Pointer Capture in the - // policy, which is updated through setPointerCaptureLocked(enabled). - bool mFocusedWindowRequestedPointerCapture GUARDED_BY(mLock); + + // The enabled state of this request is true iff the focused window on the focused display has + // requested Pointer Capture. This request also contains the sequence number associated with the + // current request. The state of this variable should always be in sync with the state of + // Pointer Capture in the policy, and is only updated through setPointerCaptureLocked(request). + PointerCaptureRequest mCurrentPointerCaptureRequest GUARDED_BY(mLock); // The window token that has Pointer Capture. // This should be in sync with PointerCaptureChangedEvents dispatched to the input channel. @@ -370,7 +372,7 @@ private: void disablePointerCaptureForcedLocked() REQUIRES(mLock); // Set the Pointer Capture state in the Policy. - void setPointerCaptureLocked(bool enabled) REQUIRES(mLock); + void setPointerCaptureLocked(bool enable) REQUIRES(mLock); // Dispatcher state at time of last ANR. std::string mLastAnrState GUARDED_BY(mLock); diff --git a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h index 219f45a7c3..fd591e0e1c 100644 --- a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h +++ b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h @@ -156,7 +156,7 @@ public: * * InputDispatcher is solely responsible for updating the Pointer Capture state. */ - virtual void setPointerCapture(bool enabled) = 0; + virtual void setPointerCapture(const PointerCaptureRequest&) = 0; /* Notifies the policy that the drag window has moved over to another window */ virtual void notifyDropWindow(const sp<IBinder>& token, float x, float y) = 0; diff --git a/services/inputflinger/docs/pointer_capture.md b/services/inputflinger/docs/pointer_capture.md index 8da699dc56..0b44187c08 100644 --- a/services/inputflinger/docs/pointer_capture.md +++ b/services/inputflinger/docs/pointer_capture.md @@ -17,6 +17,8 @@ `InputDispatcher` is responsible for controlling the state of Pointer Capture. Since the feature requires changes to how events are generated, Pointer Capture is configured in `InputReader`. +We use a sequence number to synchronize different requests to enable Pointer Capture between InputReader and InputDispatcher. + ### Enabling Pointer Capture There are four key steps that take place when Pointer Capture is enabled: @@ -40,5 +42,5 @@ When an `InputWindow` with Pointer Capture loses focus, Pointer Capture is disab `InputDispatcher` tracks two pieces of state information regarding Pointer Capture: -- `mFocusedWindowRequestedPointerCapture`: Whether or not the focused window has requested Pointer Capture. This is updated whenever the Dispatcher receives requests from `InputManagerService`. +- `mCurrentPointerCaptureRequest`: The sequence number of the current Pointer Capture request. This request is enabled iff the focused window has requested Pointer Capture. This is updated whenever the Dispatcher receives requests from `InputManagerService`. - `mWindowTokenWithPointerCapture`: The Binder token of the `InputWindow` that currently has Pointer Capture. This is only updated during the dispatch cycle. If it is not `nullptr`, it signifies that the window was notified that it has Pointer Capture. diff --git a/services/inputflinger/include/InputListener.h b/services/inputflinger/include/InputListener.h index 4b7d26df2b..fe74214b36 100644 --- a/services/inputflinger/include/InputListener.h +++ b/services/inputflinger/include/InputListener.h @@ -211,11 +211,12 @@ struct NotifyDeviceResetArgs : public NotifyArgs { /* Describes a change in the state of Pointer Capture. */ struct NotifyPointerCaptureChangedArgs : public NotifyArgs { - bool enabled; + // The sequence number of the Pointer Capture request, if enabled. + PointerCaptureRequest request; inline NotifyPointerCaptureChangedArgs() {} - NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, bool enabled); + NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, const PointerCaptureRequest&); NotifyPointerCaptureChangedArgs(const NotifyPointerCaptureChangedArgs& other); diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h index 7fdbbfdac4..3c8ac1cf7b 100644 --- a/services/inputflinger/include/InputReaderBase.h +++ b/services/inputflinger/include/InputReaderBase.h @@ -279,29 +279,30 @@ struct InputReaderConfiguration { // True to show the location of touches on the touch screen as spots. bool showTouches; - // True if pointer capture is enabled. - bool pointerCapture; + // The latest request to enable or disable Pointer Capture. + PointerCaptureRequest pointerCaptureRequest; // The set of currently disabled input devices. std::set<int32_t> disabledDevices; - InputReaderConfiguration() : - virtualKeyQuietTime(0), + InputReaderConfiguration() + : virtualKeyQuietTime(0), pointerVelocityControlParameters(1.0f, 500.0f, 3000.0f, 3.0f), wheelVelocityControlParameters(1.0f, 15.0f, 50.0f, 4.0f), pointerGesturesEnabled(true), - pointerGestureQuietInterval(100 * 1000000LL), // 100 ms - pointerGestureDragMinSwitchSpeed(50), // 50 pixels per second - pointerGestureTapInterval(150 * 1000000LL), // 150 ms - pointerGestureTapDragInterval(150 * 1000000LL), // 150 ms - pointerGestureTapSlop(10.0f), // 10 pixels + pointerGestureQuietInterval(100 * 1000000LL), // 100 ms + pointerGestureDragMinSwitchSpeed(50), // 50 pixels per second + pointerGestureTapInterval(150 * 1000000LL), // 150 ms + pointerGestureTapDragInterval(150 * 1000000LL), // 150 ms + pointerGestureTapSlop(10.0f), // 10 pixels pointerGestureMultitouchSettleInterval(100 * 1000000LL), // 100 ms - pointerGestureMultitouchMinDistance(15), // 15 pixels - pointerGestureSwipeTransitionAngleCosine(0.2588f), // cosine of 75 degrees + pointerGestureMultitouchMinDistance(15), // 15 pixels + pointerGestureSwipeTransitionAngleCosine(0.2588f), // cosine of 75 degrees pointerGestureSwipeMaxWidthRatio(0.25f), pointerGestureMovementSpeedRatio(0.8f), pointerGestureZoomSpeedRatio(0.3f), - showTouches(false), pointerCapture(false) { } + showTouches(false), + pointerCaptureRequest() {} static std::string changesToString(uint32_t changes); diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 10c04f606c..5120860503 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -367,9 +367,15 @@ void InputReader::refreshConfigurationLocked(uint32_t changes) { } if (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE) { - const NotifyPointerCaptureChangedArgs args(mContext.getNextId(), now, - mConfig.pointerCapture); - mQueuedListener->notifyPointerCaptureChanged(&args); + if (mCurrentPointerCaptureRequest == mConfig.pointerCaptureRequest) { + ALOGV("Skipping notifying pointer capture changes: " + "There was no change in the pointer capture state."); + } else { + mCurrentPointerCaptureRequest = mConfig.pointerCaptureRequest; + const NotifyPointerCaptureChangedArgs args(mContext.getNextId(), now, + mCurrentPointerCaptureRequest); + mQueuedListener->notifyPointerCaptureChanged(&args); + } } } diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h index a00c5af4dc..e44aa0fe27 100644 --- a/services/inputflinger/reader/include/InputReader.h +++ b/services/inputflinger/reader/include/InputReader.h @@ -230,6 +230,8 @@ private: uint32_t mConfigurationChangesToRefresh GUARDED_BY(mLock); void refreshConfigurationLocked(uint32_t changes) REQUIRES(mLock); + PointerCaptureRequest mCurrentPointerCaptureRequest GUARDED_BY(mLock); + // state queries typedef int32_t (InputDevice::*GetStateFunc)(uint32_t sourceMask, int32_t code); int32_t getStateLocked(int32_t deviceId, uint32_t sourceMask, int32_t code, diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.cpp b/services/inputflinger/reader/mapper/CursorInputMapper.cpp index 437902a79b..2ac41b1e67 100644 --- a/services/inputflinger/reader/mapper/CursorInputMapper.cpp +++ b/services/inputflinger/reader/mapper/CursorInputMapper.cpp @@ -154,9 +154,9 @@ void CursorInputMapper::configure(nsecs_t when, const InputReaderConfiguration* mHWheelScale = 1.0f; } - if ((!changes && config->pointerCapture) || + if ((!changes && config->pointerCaptureRequest.enable) || (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE)) { - if (config->pointerCapture) { + if (config->pointerCaptureRequest.enable) { if (mParameters.mode == Parameters::MODE_POINTER) { mParameters.mode = Parameters::MODE_POINTER_RELATIVE; mSource = AINPUT_SOURCE_MOUSE_RELATIVE; diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index f45731527f..ae89ca1ca9 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -603,7 +603,7 @@ void TouchInputMapper::configureSurface(nsecs_t when, bool* outResetNeeded) { // Determine device mode. if (mParameters.deviceType == Parameters::DeviceType::POINTER && - mConfig.pointerGesturesEnabled && !mConfig.pointerCapture) { + mConfig.pointerGesturesEnabled && !mConfig.pointerCaptureRequest.enable) { mSource = AINPUT_SOURCE_MOUSE; mDeviceMode = DeviceMode::POINTER; if (hasStylus()) { @@ -776,11 +776,12 @@ void TouchInputMapper::configureSurface(nsecs_t when, bool* outResetNeeded) { // preserve the cursor position. if (mDeviceMode == DeviceMode::POINTER || (mDeviceMode == DeviceMode::DIRECT && mConfig.showTouches) || - (mParameters.deviceType == Parameters::DeviceType::POINTER && mConfig.pointerCapture)) { + (mParameters.deviceType == Parameters::DeviceType::POINTER && + mConfig.pointerCaptureRequest.enable)) { if (mPointerController == nullptr) { mPointerController = getContext()->getPointerController(getDeviceId()); } - if (mConfig.pointerCapture) { + if (mConfig.pointerCaptureRequest.enable) { mPointerController->fade(PointerControllerInterface::Transition::IMMEDIATE); } } else { diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index dff0752b59..7d4c6386f8 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -239,19 +239,22 @@ public: mConfig.keyRepeatDelay = delay; } - void waitForSetPointerCapture(bool enabled) { + PointerCaptureRequest assertSetPointerCaptureCalled(bool enabled) { std::unique_lock lock(mLock); base::ScopedLockAssertion assumeLocked(mLock); if (!mPointerCaptureChangedCondition.wait_for(lock, 100ms, [this, enabled]() REQUIRES(mLock) { - return mPointerCaptureEnabled && - *mPointerCaptureEnabled == + return mPointerCaptureRequest->enable == enabled; })) { - FAIL() << "Timed out waiting for setPointerCapture(" << enabled << ") to be called."; + ADD_FAILURE() << "Timed out waiting for setPointerCapture(" << enabled + << ") to be called."; + return {}; } - mPointerCaptureEnabled.reset(); + auto request = *mPointerCaptureRequest; + mPointerCaptureRequest.reset(); + return request; } void assertSetPointerCaptureNotCalled() { @@ -259,11 +262,11 @@ public: base::ScopedLockAssertion assumeLocked(mLock); if (mPointerCaptureChangedCondition.wait_for(lock, 100ms) != std::cv_status::timeout) { - FAIL() << "Expected setPointerCapture(enabled) to not be called, but was called. " + FAIL() << "Expected setPointerCapture(request) to not be called, but was called. " "enabled = " - << *mPointerCaptureEnabled; + << std::to_string(mPointerCaptureRequest->enable); } - mPointerCaptureEnabled.reset(); + mPointerCaptureRequest.reset(); } void assertDropTargetEquals(const sp<IBinder>& targetToken) { @@ -281,7 +284,8 @@ private: std::optional<NotifySwitchArgs> mLastNotifySwitch GUARDED_BY(mLock); std::condition_variable mPointerCaptureChangedCondition; - std::optional<bool> mPointerCaptureEnabled GUARDED_BY(mLock); + + std::optional<PointerCaptureRequest> mPointerCaptureRequest GUARDED_BY(mLock); // ANR handling std::queue<std::shared_ptr<InputApplicationHandle>> mAnrApplications GUARDED_BY(mLock); @@ -398,9 +402,9 @@ private: mOnPointerDownToken = newToken; } - void setPointerCapture(bool enabled) override { + void setPointerCapture(const PointerCaptureRequest& request) override { std::scoped_lock lock(mLock); - mPointerCaptureEnabled = {enabled}; + mPointerCaptureRequest = {request}; mPointerCaptureChangedCondition.notify_all(); } @@ -1379,8 +1383,9 @@ static NotifyMotionArgs generateMotionArgs(int32_t action, int32_t source, int32 return generateMotionArgs(action, source, displayId, {PointF{100, 200}}); } -static NotifyPointerCaptureChangedArgs generatePointerCaptureChangedArgs(bool enabled) { - return NotifyPointerCaptureChangedArgs(/* id */ 0, systemTime(SYSTEM_TIME_MONOTONIC), enabled); +static NotifyPointerCaptureChangedArgs generatePointerCaptureChangedArgs( + const PointerCaptureRequest& request) { + return NotifyPointerCaptureChangedArgs(/* id */ 0, systemTime(SYSTEM_TIME_MONOTONIC), request); } TEST_F(InputDispatcherTest, SetInputWindow_SingleWindowTouch) { @@ -4601,16 +4606,18 @@ protected: mWindow->consumeFocusEvent(true); } - void notifyPointerCaptureChanged(bool enabled) { - const NotifyPointerCaptureChangedArgs args = generatePointerCaptureChangedArgs(enabled); + void notifyPointerCaptureChanged(const PointerCaptureRequest& request) { + const NotifyPointerCaptureChangedArgs args = generatePointerCaptureChangedArgs(request); mDispatcher->notifyPointerCaptureChanged(&args); } - void requestAndVerifyPointerCapture(const sp<FakeWindowHandle>& window, bool enabled) { + PointerCaptureRequest requestAndVerifyPointerCapture(const sp<FakeWindowHandle>& window, + bool enabled) { mDispatcher->requestPointerCapture(window->getToken(), enabled); - mFakePolicy->waitForSetPointerCapture(enabled); - notifyPointerCaptureChanged(enabled); + auto request = mFakePolicy->assertSetPointerCaptureCalled(enabled); + notifyPointerCaptureChanged(request); window->consumeCaptureEvent(enabled); + return request; } }; @@ -4632,7 +4639,7 @@ TEST_F(InputDispatcherPointerCaptureTests, EnablePointerCaptureWhenFocused) { } TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLosesFocus) { - requestAndVerifyPointerCapture(mWindow, true); + auto request = requestAndVerifyPointerCapture(mWindow, true); setFocusedWindow(mSecondWindow); @@ -4640,26 +4647,26 @@ TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLose mWindow->consumeCaptureEvent(false); mWindow->consumeFocusEvent(false); mSecondWindow->consumeFocusEvent(true); - mFakePolicy->waitForSetPointerCapture(false); + mFakePolicy->assertSetPointerCaptureCalled(false); // Ensure that additional state changes from InputReader are not sent to the window. - notifyPointerCaptureChanged(false); - notifyPointerCaptureChanged(true); - notifyPointerCaptureChanged(false); + notifyPointerCaptureChanged({}); + notifyPointerCaptureChanged(request); + notifyPointerCaptureChanged({}); mWindow->assertNoEvents(); mSecondWindow->assertNoEvents(); mFakePolicy->assertSetPointerCaptureNotCalled(); } TEST_F(InputDispatcherPointerCaptureTests, UnexpectedStateChangeDisablesPointerCapture) { - requestAndVerifyPointerCapture(mWindow, true); + auto request = requestAndVerifyPointerCapture(mWindow, true); // InputReader unexpectedly disables and enables pointer capture. - notifyPointerCaptureChanged(false); - notifyPointerCaptureChanged(true); + notifyPointerCaptureChanged({}); + notifyPointerCaptureChanged(request); // Ensure that Pointer Capture is disabled. - mFakePolicy->waitForSetPointerCapture(false); + mFakePolicy->assertSetPointerCaptureCalled(false); mWindow->consumeCaptureEvent(false); mWindow->assertNoEvents(); } @@ -4669,24 +4676,43 @@ TEST_F(InputDispatcherPointerCaptureTests, OutOfOrderRequests) { // The first window loses focus. setFocusedWindow(mSecondWindow); - mFakePolicy->waitForSetPointerCapture(false); + mFakePolicy->assertSetPointerCaptureCalled(false); mWindow->consumeCaptureEvent(false); // Request Pointer Capture from the second window before the notification from InputReader // arrives. mDispatcher->requestPointerCapture(mSecondWindow->getToken(), true); - mFakePolicy->waitForSetPointerCapture(true); + auto request = mFakePolicy->assertSetPointerCaptureCalled(true); // InputReader notifies Pointer Capture was disabled (because of the focus change). - notifyPointerCaptureChanged(false); + notifyPointerCaptureChanged({}); // InputReader notifies Pointer Capture was enabled (because of mSecondWindow's request). - notifyPointerCaptureChanged(true); + notifyPointerCaptureChanged(request); mSecondWindow->consumeFocusEvent(true); mSecondWindow->consumeCaptureEvent(true); } +TEST_F(InputDispatcherPointerCaptureTests, EnableRequestFollowsSequenceNumbers) { + // App repeatedly enables and disables capture. + mDispatcher->requestPointerCapture(mWindow->getToken(), true); + auto firstRequest = mFakePolicy->assertSetPointerCaptureCalled(true); + mDispatcher->requestPointerCapture(mWindow->getToken(), false); + mFakePolicy->assertSetPointerCaptureCalled(false); + mDispatcher->requestPointerCapture(mWindow->getToken(), true); + auto secondRequest = mFakePolicy->assertSetPointerCaptureCalled(true); + + // InputReader notifies that PointerCapture has been enabled for the first request. Since the + // first request is now stale, this should do nothing. + notifyPointerCaptureChanged(firstRequest); + mWindow->assertNoEvents(); + + // InputReader notifies that the second request was enabled. + notifyPointerCaptureChanged(secondRequest); + mWindow->consumeCaptureEvent(true); +} + class InputDispatcherUntrustedTouchesTest : public InputDispatcherTest { protected: constexpr static const float MAXIMUM_OBSCURING_OPACITY = 0.8; diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index 997cbe88a1..38dfe4041f 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -299,8 +299,9 @@ public: transform = t; } - void setPointerCapture(bool enabled) { - mConfig.pointerCapture = enabled; + PointerCaptureRequest setPointerCapture(bool enabled) { + mConfig.pointerCaptureRequest = {enabled, mNextPointerCaptureSequenceNumber++}; + return mConfig.pointerCaptureRequest; } void setShowTouches(bool enabled) { @@ -314,6 +315,8 @@ public: float getPointerGestureMovementSpeedRatio() { return mConfig.pointerGestureMovementSpeedRatio; } private: + uint32_t mNextPointerCaptureSequenceNumber = 0; + DisplayViewport createDisplayViewport(int32_t displayId, int32_t width, int32_t height, int32_t orientation, bool isActive, const std::string& uniqueId, @@ -1961,24 +1964,24 @@ TEST_F(InputReaderTest, GetKeyCodeState_ForwardsRequestsToSubdeviceMappers) { TEST_F(InputReaderTest, ChangingPointerCaptureNotifiesInputListener) { NotifyPointerCaptureChangedArgs args; - mFakePolicy->setPointerCapture(true); + auto request = mFakePolicy->setPointerCapture(true); mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE); mReader->loopOnce(); mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_TRUE(args.enabled) << "Pointer Capture should be enabled."; + ASSERT_TRUE(args.request.enable) << "Pointer Capture should be enabled."; + ASSERT_EQ(args.request, request) << "Pointer Capture sequence number should match."; mFakePolicy->setPointerCapture(false); mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE); mReader->loopOnce(); mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_FALSE(args.enabled) << "Pointer Capture should be disabled."; + ASSERT_FALSE(args.request.enable) << "Pointer Capture should be disabled."; - // Verify that the Pointer Capture state is re-configured correctly when the configuration value + // Verify that the Pointer Capture state is not updated when the configuration value // does not change. mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE); mReader->loopOnce(); - mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_FALSE(args.enabled) << "Pointer Capture should be disabled."; + mFakeListener->assertNotifyCaptureWasNotCalled(); } class FakeVibratorInputMapper : public FakeInputMapper { diff --git a/services/inputflinger/tests/TestInputListener.cpp b/services/inputflinger/tests/TestInputListener.cpp index fb7de97b49..6a26c636d9 100644 --- a/services/inputflinger/tests/TestInputListener.cpp +++ b/services/inputflinger/tests/TestInputListener.cpp @@ -100,6 +100,11 @@ void TestInputListener::assertNotifyCaptureWasCalled( "to have been called.")); } +void TestInputListener::assertNotifyCaptureWasNotCalled() { + ASSERT_NO_FATAL_FAILURE(assertNotCalled<NotifyPointerCaptureChangedArgs>( + "notifyPointerCaptureChanged() should not be called.")); +} + template <class NotifyArgsType> void TestInputListener::assertCalled(NotifyArgsType* outEventArgs, std::string message) { std::unique_lock<std::mutex> lock(mLock); diff --git a/services/inputflinger/tests/TestInputListener.h b/services/inputflinger/tests/TestInputListener.h index 0ffcaaa527..0a1dc4b0b9 100644 --- a/services/inputflinger/tests/TestInputListener.h +++ b/services/inputflinger/tests/TestInputListener.h @@ -55,6 +55,7 @@ public: void assertNotifySwitchWasCalled(NotifySwitchArgs* outEventArgs = nullptr); void assertNotifyCaptureWasCalled(NotifyPointerCaptureChangedArgs* outEventArgs = nullptr); + void assertNotifyCaptureWasNotCalled(); void assertNotifySensorWasCalled(NotifySensorArgs* outEventArgs = nullptr); void assertNotifyVibratorStateWasCalled(NotifyVibratorStateArgs* outEventArgs = nullptr); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 75dd51c992..d8cead5e50 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -820,11 +820,7 @@ void Layer::setZOrderRelativeOf(const wp<Layer>& relativeOf) { } bool Layer::setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relativeZ) { - sp<Handle> handle = static_cast<Handle*>(relativeToHandle.get()); - if (handle == nullptr) { - return false; - } - sp<Layer> relative = handle->owner.promote(); + sp<Layer> relative = fromHandle(relativeToHandle).promote(); if (relative == nullptr) { return false; } @@ -1610,8 +1606,7 @@ void Layer::setChildrenDrawingParent(const sp<Layer>& newParent) { bool Layer::reparent(const sp<IBinder>& newParentHandle) { sp<Layer> newParent; if (newParentHandle != nullptr) { - auto handle = static_cast<Handle*>(newParentHandle.get()); - newParent = handle->owner.promote(); + newParent = fromHandle(newParentHandle).promote(); if (newParent == nullptr) { ALOGE("Unable to promote Layer handle"); return false; @@ -1986,24 +1981,10 @@ void Layer::commitChildList() { mDrawingParent = mCurrentParent; } -static wp<Layer> extractLayerFromBinder(const wp<IBinder>& weakBinderHandle) { - if (weakBinderHandle == nullptr) { - return nullptr; - } - sp<IBinder> binderHandle = weakBinderHandle.promote(); - if (binderHandle == nullptr) { - return nullptr; - } - sp<Layer::Handle> handle = static_cast<Layer::Handle*>(binderHandle.get()); - if (handle == nullptr) { - return nullptr; - } - return handle->owner; -} void Layer::setInputInfo(const InputWindowInfo& info) { mDrawingState.inputInfo = info; - mDrawingState.touchableRegionCrop = extractLayerFromBinder(info.touchableRegionCropHandle); + mDrawingState.touchableRegionCrop = fromHandle(info.touchableRegionCropHandle.promote()); mDrawingState.modified = true; mFlinger->mInputInfoChanged = true; setTransactionFlags(eTransactionNeeded); @@ -2562,6 +2543,23 @@ void Layer::setClonedChild(const sp<Layer>& clonedChild) { mFlinger->mNumClones++; } +const String16 Layer::Handle::kDescriptor = String16("android.Layer.Handle"); + +wp<Layer> Layer::fromHandle(const sp<IBinder>& handleBinder) { + if (handleBinder == nullptr) { + return nullptr; + } + + BBinder* b = handleBinder->localBinder(); + if (b == nullptr || b->getInterfaceDescriptor() != Handle::kDescriptor) { + return nullptr; + } + + // We can safely cast this binder since its local and we verified its interface descriptor. + sp<Handle> handle = static_cast<Handle*>(handleBinder.get()); + return handle->owner; +} + // --------------------------------------------------------------------------- std::ostream& operator<<(std::ostream& stream, const Layer::FrameRate& rate) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 59f5b0dc73..8905548b64 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -289,16 +289,17 @@ public: class LayerCleaner { sp<SurfaceFlinger> mFlinger; sp<Layer> mLayer; + BBinder* mHandle; protected: ~LayerCleaner() { // destroy client resources - mFlinger->onHandleDestroyed(mLayer); + mFlinger->onHandleDestroyed(mHandle, mLayer); } public: - LayerCleaner(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer) - : mFlinger(flinger), mLayer(layer) {} + LayerCleaner(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer, BBinder* handle) + : mFlinger(flinger), mLayer(layer), mHandle(handle) {} }; /* @@ -312,11 +313,15 @@ public: class Handle : public BBinder, public LayerCleaner { public: Handle(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer) - : LayerCleaner(flinger, layer), owner(layer) {} + : LayerCleaner(flinger, layer, this), owner(layer) {} + const String16& getInterfaceDescriptor() const override { return kDescriptor; } + static const String16 kDescriptor; wp<Layer> owner; }; + static wp<Layer> fromHandle(const sp<IBinder>& handle); + explicit Layer(const LayerCreationArgs& args); virtual ~Layer(); diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp index 0334d70bd5..73b7b6365c 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp @@ -911,7 +911,10 @@ RefreshRateConfigs::KernelIdleTimerAction RefreshRateConfigs::getIdleTimerAction int RefreshRateConfigs::getFrameRateDivider(Fps displayFrameRate, Fps layerFrameRate) { // This calculation needs to be in sync with the java code // in DisplayManagerService.getDisplayInfoForFrameRateOverride - constexpr float kThreshold = 0.1f; + + // The threshold must be smaller than 0.001 in order to differentiate + // between the fractional pairs (e.g. 59.94 and 60). + constexpr float kThreshold = 0.0009f; const auto numPeriods = displayFrameRate.getValue() / layerFrameRate.getValue(); const auto numPeriodsRounded = std::round(numPeriods); if (std::abs(numPeriods - numPeriodsRounded) > kThreshold) { diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index e0b364020b..64ad178b26 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -386,6 +386,13 @@ void Scheduler::dispatchCachedReportedMode() { } const auto modeId = *mFeatures.modeId; + // If the modeId is not the current mode, this means that a + // mode change is in progress. In that case we shouldn't dispatch an event + // as it will be dispatched when the current mode changes. + if (mRefreshRateConfigs.getCurrentRefreshRate().getModeId() != modeId) { + return; + } + const auto vsyncPeriod = mRefreshRateConfigs.getRefreshRateFromModeId(modeId).getVsyncPeriod(); // If there is no change from cached mode, there is no need to dispatch an event diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index abc49bf574..6b5094fd21 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3622,7 +3622,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( sp<Layer> layer = nullptr; if (s.surface) { - layer = fromHandleLocked(s.surface).promote(); + layer = fromHandle(s.surface).promote(); } else if (s.hasBufferChanges()) { ALOGW("Transaction with buffer, but no Layer?"); continue; @@ -3789,7 +3789,7 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp, postTime, permissions, listenerCallbacksWithSurfaces); if ((flags & eAnimation) && state.state.surface) { - if (const auto layer = fromHandleLocked(state.state.surface).promote(); layer) { + if (const auto layer = fromHandle(state.state.surface).promote(); layer) { mScheduler->recordLayerHistory(layer.get(), isAutoTimestamp ? 0 : desiredPresentTime, LayerHistory::LayerUpdateType::AnimationTX); @@ -3944,13 +3944,11 @@ uint32_t SurfaceFlinger::setClientStateLocked( if (what & layer_state_t::eLayerCreated) { layer = handleLayerCreatedLocked(s.surface); if (layer) { - // put the created layer into mLayersByLocalBinderToken. - mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer); flags |= eTransactionNeeded | eTraversalNeeded; mLayersAdded = true; } } else { - layer = fromHandleLocked(s.surface).promote(); + layer = fromHandle(s.surface).promote(); } } else { // The client may provide us a null handle. Treat it as if the layer was removed. @@ -4278,7 +4276,7 @@ status_t SurfaceFlinger::mirrorLayer(const sp<Client>& client, const sp<IBinder> { Mutex::Autolock _l(mStateLock); - mirrorFrom = fromHandleLocked(mirrorFromHandle).promote(); + mirrorFrom = fromHandle(mirrorFromHandle).promote(); if (!mirrorFrom) { return NAME_NOT_FOUND; } @@ -4476,7 +4474,7 @@ void SurfaceFlinger::markLayerPendingRemovalLocked(const sp<Layer>& layer) { setTransactionFlags(eTransactionNeeded); } -void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) { +void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer) { Mutex::Autolock lock(mStateLock); // If a layer has a parent, we allow it to out-live it's handle // with the idea that the parent holds a reference and will eventually @@ -4487,17 +4485,7 @@ void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) { mCurrentState.layersSortedByZ.remove(layer); } markLayerPendingRemovalLocked(layer); - - auto it = mLayersByLocalBinderToken.begin(); - while (it != mLayersByLocalBinderToken.end()) { - if (it->second == layer) { - mBufferCountTracker.remove(it->first->localBinder()); - it = mLayersByLocalBinderToken.erase(it); - } else { - it++; - } - } - + mBufferCountTracker.remove(handle); layer.clear(); } @@ -6101,7 +6089,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, { Mutex::Autolock lock(mStateLock); - parent = fromHandleLocked(args.layerHandle).promote(); + parent = fromHandle(args.layerHandle).promote(); if (parent == nullptr || parent->isRemovedFromCurrentState()) { ALOGE("captureLayers called with an invalid or removed parent"); return NAME_NOT_FOUND; @@ -6132,7 +6120,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, reqSize = ui::Size(crop.width() * args.frameScaleX, crop.height() * args.frameScaleY); for (const auto& handle : args.excludeHandles) { - sp<Layer> excludeLayer = fromHandleLocked(handle).promote(); + sp<Layer> excludeLayer = fromHandle(handle).promote(); if (excludeLayer != nullptr) { excludeLayers.emplace(excludeLayer); } else { @@ -6628,24 +6616,8 @@ status_t SurfaceFlinger::getDesiredDisplayModeSpecs(const sp<IBinder>& displayTo } } -wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) { - Mutex::Autolock _l(mStateLock); - return fromHandleLocked(handle); -} - -wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) const { - BBinder* b = nullptr; - if (handle) { - b = handle->localBinder(); - } - if (b == nullptr) { - return nullptr; - } - auto it = mLayersByLocalBinderToken.find(b); - if (it != mLayersByLocalBinderToken.end()) { - return it->second; - } - return nullptr; +wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) const { + return Layer::fromHandle(handle); } void SurfaceFlinger::onLayerFirstRef(Layer* layer) { @@ -6950,7 +6922,7 @@ sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle) { sp<Layer> parent; bool allowAddRoot = state->addToRoot; if (state->initialParent != nullptr) { - parent = fromHandleLocked(state->initialParent.promote()).promote(); + parent = fromHandle(state->initialParent.promote()).promote(); if (parent == nullptr) { ALOGE("Invalid parent %p", state->initialParent.unsafe_get()); allowAddRoot = false; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index fa19b7f1bb..4e8e614cbd 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -327,8 +327,7 @@ public: // Returns nullptr if the handle does not point to an existing layer. // Otherwise, returns a weak reference so that callers off the main-thread // won't accidentally hold onto the last strong reference. - wp<Layer> fromHandle(const sp<IBinder>& handle); - wp<Layer> fromHandleLocked(const sp<IBinder>& handle) const REQUIRES(mStateLock); + wp<Layer> fromHandle(const sp<IBinder>& handle) const; // If set, disables reusing client composition buffers. This can be set by // debug.sf.disable_client_composition_cache @@ -899,7 +898,7 @@ private: // called when all clients have released all their references to // this layer meaning it is entirely safe to destroy all // resources associated to this layer. - void onHandleDestroyed(sp<Layer>& layer); + void onHandleDestroyed(BBinder* handle, sp<Layer>& layer); void markLayerPendingRemovalLocked(const sp<Layer>& layer); // add a layer to SurfaceFlinger @@ -1291,8 +1290,6 @@ private: std::optional<DisplayIdGenerator<HalVirtualDisplayId>> hal; } mVirtualDisplayIdGenerators; - std::unordered_map<BBinder*, wp<Layer>> mLayersByLocalBinderToken GUARDED_BY(mStateLock); - // don't use a lock for these, we don't care int mDebugRegion = 0; bool mDebugDisableHWC = false; diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp index 23ab7c8bab..837b2e8924 100644 --- a/services/surfaceflinger/SurfaceInterceptor.cpp +++ b/services/surfaceflinger/SurfaceInterceptor.cpp @@ -183,12 +183,9 @@ status_t SurfaceInterceptor::writeProtoFileLocked() { return NO_ERROR; } -const sp<const Layer> SurfaceInterceptor::getLayer(const wp<const IBinder>& weakHandle) const { - const sp<const IBinder>& handle(weakHandle.promote()); - const auto layerHandle(static_cast<const Layer::Handle*>(handle.get())); - const sp<const Layer> layer(layerHandle->owner.promote()); - // layer could be a nullptr at this point - return layer; +const sp<const Layer> SurfaceInterceptor::getLayer(const wp<IBinder>& weakHandle) const { + sp<IBinder> handle = weakHandle.promote(); + return Layer::fromHandle(handle).promote(); } int32_t SurfaceInterceptor::getLayerId(const sp<const Layer>& layer) const { @@ -203,12 +200,11 @@ int32_t SurfaceInterceptor::getLayerIdFromWeakRef(const wp<const Layer>& layer) return strongLayer == nullptr ? -1 : getLayerId(strongLayer); } -int32_t SurfaceInterceptor::getLayerIdFromHandle(const sp<const IBinder>& handle) const { +int32_t SurfaceInterceptor::getLayerIdFromHandle(const sp<IBinder>& handle) const { if (handle == nullptr) { return -1; } - const auto layerHandle(static_cast<const Layer::Handle*>(handle.get())); - const sp<const Layer> layer(layerHandle->owner.promote()); + const sp<const Layer> layer = Layer::fromHandle(handle).promote(); return layer == nullptr ? -1 : getLayerId(layer); } diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h index 673f9e789d..c9555969dc 100644 --- a/services/surfaceflinger/SurfaceInterceptor.h +++ b/services/surfaceflinger/SurfaceInterceptor.h @@ -133,10 +133,10 @@ private: void addInitialDisplayStateLocked(Increment* increment, const DisplayDeviceState& display); status_t writeProtoFileLocked(); - const sp<const Layer> getLayer(const wp<const IBinder>& weakHandle) const; + const sp<const Layer> getLayer(const wp<IBinder>& weakHandle) const; int32_t getLayerId(const sp<const Layer>& layer) const; int32_t getLayerIdFromWeakRef(const wp<const Layer>& layer) const; - int32_t getLayerIdFromHandle(const sp<const IBinder>& weakHandle) const; + int32_t getLayerIdFromHandle(const sp<IBinder>& weakHandle) const; Increment* createTraceIncrementLocked(); void addSurfaceCreationLocked(Increment* increment, const sp<const Layer>& layer); diff --git a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp index 3423bd590b..3b2bd818b7 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp @@ -2123,7 +2123,11 @@ TEST_F(RefreshRateConfigsTest, getFrameRateDivider) { refreshRateConfigs->setCurrentModeId(HWC_CONFIG_ID_90); displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps(); EXPECT_EQ(4, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, Fps(22.5f))); - EXPECT_EQ(4, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, Fps(22.6f))); + + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(24.f), Fps(25.f))); + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(24.f), Fps(23.976f))); + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(30.f), Fps(29.97f))); + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(60.f), Fps(59.94f))); } TEST_F(RefreshRateConfigsTest, getFrameRateOverrides_noLayers) { diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp index 8c54a0efe0..6191063894 100644 --- a/vulkan/libvulkan/swapchain.cpp +++ b/vulkan/libvulkan/swapchain.cpp @@ -1205,18 +1205,26 @@ VkResult CreateSwapchainKHR(VkDevice device, swap_interval ? create_info->minImageCount : mailbox_num_images; uint32_t num_images = requested_images - 1 + min_undequeued_buffers; - // Lower layer insists that we have at least two buffers. This is wasteful - // and we'd like to relax it in the shared case, but not all the pieces are - // in place for that to work yet. Note we only lie to the lower layer-- we - // don't want to give the app back a swapchain with extra images (which they - // can't actually use!). - err = native_window_set_buffer_count(window, std::max(2u, num_images)); + // Lower layer insists that we have at least min_undequeued_buffers + 1 + // buffers. This is wasteful and we'd like to relax it in the shared case, + // but not all the pieces are in place for that to work yet. Note we only + // lie to the lower layer--we don't want to give the app back a swapchain + // with extra images (which they can't actually use!). + uint32_t min_buffer_count = min_undequeued_buffers + 1; + err = native_window_set_buffer_count( + window, std::max(min_buffer_count, num_images)); if (err != android::OK) { ALOGE("native_window_set_buffer_count(%d) failed: %s (%d)", num_images, strerror(-err), err); return VK_ERROR_SURFACE_LOST_KHR; } + // In shared mode the num_images must be one regardless of how many + // buffers were allocated for the buffer queue. + if (swapchain_image_usage & VK_SWAPCHAIN_IMAGE_USAGE_SHARED_BIT_ANDROID) { + num_images = 1; + } + int32_t legacy_usage = 0; if (dispatch.GetSwapchainGrallocUsage2ANDROID) { uint64_t consumer_usage, producer_usage; |