From be392fc69318d376f18eed7c8ef5ded0d1dc8648 Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Tue, 11 Jul 2023 09:05:27 -0700 Subject: [PATCH] Revert "Listen on property_service_for_system socket" This reverts commit 90879edeea0b2fd1e766428693d736163f39c511. These fixes for b/262208935 introduced a race condition. We believe the race is fixed by ag/23879563, but at this point in the release feel that reverting the fixes and refixing in main is the better solution Test: Builds, boots Bug: 283202477 Bug: 288991737 Ignore-AOSP-First: Reverting CL only in internal Change-Id: I28491e90847f6aa0c9767b27e1d99190637048b9 --- init/property_service.cpp | 53 ++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/init/property_service.cpp b/init/property_service.cpp index 2d084db46..8da69822c 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -57,12 +57,12 @@ #include #include #include -#include #include #include #include #include #include + #include "debug_ramdisk.h" #include "epoll.h" #include "init.h" @@ -111,13 +111,12 @@ constexpr auto API_LEVEL_CURRENT = 10000; static bool persistent_properties_loaded = false; +static int property_set_fd = -1; static int from_init_socket = -1; static int init_socket = -1; static bool accept_messages = false; static std::mutex accept_messages_lock; static std::thread property_service_thread; -static std::thread property_service_for_system_thread; -static std::mutex set_property_lock; static std::unique_ptr persist_write_thread; @@ -395,7 +394,6 @@ static std::optional PropertySet(const std::string& name, const std::s return {PROP_ERROR_INVALID_VALUE}; } - auto lock = std::lock_guard{set_property_lock}; prop_info* pi = (prop_info*)__system_property_find(name.c_str()); if (pi != nullptr) { // ro.* properties are actually "write-once". @@ -581,10 +579,10 @@ uint32_t HandlePropertySetNoSocket(const std::string& name, const std::string& v return *ret; } -static void handle_property_set_fd(int fd) { +static void handle_property_set_fd() { static constexpr uint32_t kDefaultSocketTimeout = 2000; /* ms */ - int s = accept4(fd, nullptr, nullptr, SOCK_CLOEXEC); + int s = accept4(property_set_fd, nullptr, nullptr, SOCK_CLOEXEC); if (s == -1) { return; } @@ -1421,21 +1419,19 @@ static void HandleInitSocket() { } } -static void PropertyServiceThread(int fd, bool listen_init) { +static void PropertyServiceThread() { Epoll epoll; if (auto result = epoll.Open(); !result.ok()) { LOG(FATAL) << result.error(); } - if (auto result = epoll.RegisterHandler(fd, std::bind(handle_property_set_fd, fd)); + if (auto result = epoll.RegisterHandler(property_set_fd, handle_property_set_fd); !result.ok()) { LOG(FATAL) << result.error(); } - if (listen_init) { - if (auto result = epoll.RegisterHandler(init_socket, HandleInitSocket); !result.ok()) { - LOG(FATAL) << result.error(); - } + if (auto result = epoll.RegisterHandler(init_socket, HandleInitSocket); !result.ok()) { + LOG(FATAL) << result.error(); } while (true) { @@ -1486,23 +1482,6 @@ void PersistWriteThread::Write(std::string name, std::string value, SocketConnec cv_.notify_all(); } -void StartThread(const char* name, int mode, int gid, std::thread& t, bool listen_init) { - int fd = -1; - if (auto result = CreateSocket(name, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, - /*passcred=*/false, /*should_listen=*/false, mode, /*uid=*/0, - /*gid=*/gid, /*socketcon=*/{}); - result.ok()) { - fd = *result; - } else { - LOG(FATAL) << "start_property_service socket creation failed: " << result.error(); - } - - listen(fd, 8); - - auto new_thread = std::thread(PropertyServiceThread, fd, listen_init); - t.swap(new_thread); -} - void StartPropertyService(int* epoll_socket) { InitPropertySet("ro.property_service.version", "2"); @@ -1514,9 +1493,19 @@ void StartPropertyService(int* epoll_socket) { init_socket = sockets[1]; StartSendingMessages(); - StartThread(PROP_SERVICE_FOR_SYSTEM_NAME, 0660, AID_SYSTEM, property_service_for_system_thread, - true); - StartThread(PROP_SERVICE_NAME, 0666, 0, property_service_thread, false); + if (auto result = CreateSocket(PROP_SERVICE_NAME, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, + /*passcred=*/false, /*should_listen=*/false, 0666, /*uid=*/0, + /*gid=*/0, /*socketcon=*/{}); + result.ok()) { + property_set_fd = *result; + } else { + LOG(FATAL) << "start_property_service socket creation failed: " << result.error(); + } + + listen(property_set_fd, 8); + + auto new_thread = std::thread{PropertyServiceThread}; + property_service_thread.swap(new_thread); auto async_persist_writes = android::base::GetBoolProperty("ro.property_service.async_persist_writes", false);