From 71bdf2820ee0fbf698840f84fdd1255dbf8d3aee Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 17 Jun 2019 15:47:09 -0700 Subject: [PATCH] init: Handle properties in the background of calling fs_mgr It's been a long standing problem that init calls fs_mgr functions synchronously and therefore stops handling properties, which causes deadlocks if either fs_mgr, or vdc, or vold attempt to set properties. Previous work, b/21904461, shows that there is a large performance penalty for adding any amount of locking to properties, so moving property service into its own thread generically is not a viable option. However, we can be sure that init is not setting properties while the fs_mgr functions are running, so we can poll the property socket in a thread while we call these functions. The other alternative would have been to separate the fs_mgr functions into smaller pieces and revisit the main init loop between each piece. Unfortunately, this would be difficult, since fs_mgr_mount_all() calls out to different processes via logwrapper, which synchronously polls on a logging FD from the child, among other complexities that would make this strategy much more difficult than it would be worth. Bug: 21904461 Test: device boots, including when setting property in fs_mgr_mount_all() Change-Id: Ib0b7123024035884f9d90f9b489c1e2f5a2e1707 --- init/builtins.cpp | 27 ++++++++++++++++++++------ init/property_service.cpp | 41 ++++++++++++++++++++++++++++++++++++++- init/property_service.h | 10 ++++++++++ 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index 44cac4b7d..d9b1b8530 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -588,7 +588,12 @@ static Result do_mount_all(const BuiltinArguments& args) { if (!ReadFstabFromFile(fstab_file, &fstab)) { return Error() << "Could not read fstab"; } - auto mount_fstab_return_code = fs_mgr_mount_all(&fstab, mount_mode); + + auto mount_fstab_return_code = + CallFunctionAndHandleProperties(fs_mgr_mount_all, &fstab, mount_mode); + if (!mount_fstab_return_code) { + return Error() << "Could not call fs_mgr_mount_all(): " << mount_fstab_return_code.error(); + } property_set(prop_name, std::to_string(t.duration().count())); if (import_rc && SelinuxGetVendorAndroidVersion() <= __ANDROID_API_Q__) { @@ -599,7 +604,7 @@ static Result do_mount_all(const BuiltinArguments& args) { if (queue_event) { /* queue_fs_event will queue event based on mount_fstab return code * and return processed return code*/ - auto queue_fs_result = queue_fs_event(mount_fstab_return_code); + auto queue_fs_result = queue_fs_event(*mount_fstab_return_code); if (!queue_fs_result) { return Error() << "queue_fs_event() failed: " << queue_fs_result.error(); } @@ -615,8 +620,13 @@ static Result do_umount_all(const BuiltinArguments& args) { return Error() << "Could not read fstab"; } - if (auto result = fs_mgr_umount_all(&fstab); result != 0) { - return Error() << "umount_fstab() failed " << result; + auto result = CallFunctionAndHandleProperties(fs_mgr_umount_all, &fstab); + if (!result) { + return Error() << "Could not call fs_mgr_mount_all() " << result.error(); + } + + if (*result != 0) { + return Error() << "fs_mgr_mount_all() failed: " << *result; } return {}; } @@ -627,8 +637,13 @@ static Result do_swapon_all(const BuiltinArguments& args) { return Error() << "Could not read fstab '" << args[1] << "'"; } - if (!fs_mgr_swapon_all(fstab)) { - return Error() << "fs_mgr_swapon_all() failed"; + auto result = CallFunctionAndHandleProperties(fs_mgr_swapon_all, fstab); + if (!result) { + return Error() << "Could not call fs_mgr_swapon_all() " << result.error(); + } + + if (*result == 0) { + return Error() << "fs_mgr_swapon_all() failed."; } return {}; diff --git a/init/property_service.cpp b/init/property_service.cpp index 14bb8198e..1520c9f33 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -39,6 +39,7 @@ #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ #include +#include #include #include #include @@ -52,6 +53,7 @@ #include #include #include +#include #include #include #include @@ -59,7 +61,6 @@ #include #include "debug_ramdisk.h" -#include "epoll.h" #include "init.h" #include "persistent_properties.h" #include "property_type.h" @@ -76,6 +77,7 @@ using android::base::StartsWith; using android::base::StringPrintf; using android::base::Timer; using android::base::Trim; +using android::base::unique_fd; using android::base::WriteStringToFile; using android::properties::BuildTrie; using android::properties::ParsePropertyInfoFile; @@ -1006,5 +1008,42 @@ void StartPropertyService(Epoll* epoll) { } } +Result CallFunctionAndHandlePropertiesImpl(const std::function& f) { + unique_fd reader; + unique_fd writer; + if (!Socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, &reader, &writer)) { + return ErrnoError() << "Could not create socket pair"; + } + + int result = 0; + std::atomic end = false; + auto thread = std::thread{[&f, &result, &end, &writer] { + result = f(); + end = true; + send(writer, "1", 1, 0); + }}; + + Epoll epoll; + if (auto result = epoll.Open(); !result) { + return Error() << "Could not create epoll: " << result.error(); + } + if (auto result = epoll.RegisterHandler(property_set_fd, handle_property_set_fd); !result) { + return Error() << "Could not register epoll handler for property fd: " << result.error(); + } + + // No-op function, just used to break from loop. + if (auto result = epoll.RegisterHandler(reader, [] {}); !result) { + return Error() << "Could not register epoll handler for ending thread:" << result.error(); + } + + while (!end) { + epoll.Wait({}); + } + + thread.join(); + + return result; +} + } // namespace init } // namespace android diff --git a/init/property_service.h b/init/property_service.h index 7f9f84422..dc47b4db8 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -18,9 +18,11 @@ #include +#include #include #include "epoll.h" +#include "result.h" namespace android { namespace init { @@ -37,5 +39,13 @@ void property_load_boot_defaults(bool load_debug_prop); void load_persist_props(); void StartPropertyService(Epoll* epoll); +template +Result CallFunctionAndHandleProperties(F&& f, Args&&... args) { + Result CallFunctionAndHandlePropertiesImpl(const std::function& f); + + auto func = [&] { return f(args...); }; + return CallFunctionAndHandlePropertiesImpl(func); +} + } // namespace init } // namespace android