Handle concurrent exit and wake lock acquisition.
We use "-Wexit-time-destructors" and "-fno-c++-static-destructors" flags
to make sure that no static variables are destroyed at exit time.
We do this to avoid the race condition between process exit and use of
static vars when calling acquire/release_wake_lock().
Bug: 117575503
Test: libpower_test
Change-Id: I16acfeb2412685e376d8bd1069c86f60f56d215a
diff --git a/Android.bp b/Android.bp
index d8c42ac..60f2468 100644
--- a/Android.bp
+++ b/Android.bp
@@ -9,9 +9,18 @@
export_header_lib_headers: ["libcutils_headers"],
}
+cc_defaults {
+ name: "libpower_defaults",
+ defaults: ["system_suspend_defaults"],
+ cflags: [
+ "-Wexit-time-destructors",
+ "-fno-c++-static-destructors",
+ ],
+}
+
cc_library {
name: "libpower",
- defaults: ["system_suspend_defaults"],
+ defaults: ["libpower_defaults"],
srcs: ["power.cpp"],
export_include_dirs: ["include"],
shared_libs: ["android.system.suspend@1.0"],
@@ -23,14 +32,15 @@
cc_test {
name: "libpower_test",
- defaults: ["system_suspend_defaults"],
+ defaults: ["libpower_defaults"],
srcs: ["power_test.cpp"],
- shared_libs: ["libpower"],
+ static_libs: ["libpower"],
+ shared_libs: ["android.system.suspend@1.0"],
}
cc_library_shared {
name: "libhardware_legacy",
- defaults: ["system_suspend_defaults"],
+ defaults: ["libpower_defaults"],
vendor_available: true,
vndk: {
enabled: true,
diff --git a/power.cpp b/power.cpp
index 093fd4c..c4f2036 100644
--- a/power.cpp
+++ b/power.cpp
@@ -18,7 +18,6 @@
#define ATRACE_TAG ATRACE_TAG_POWER
#include <android-base/logging.h>
-#include <android/system/suspend/1.0/BpHwSystemSuspend.h>
#include <android/system/suspend/1.0/ISystemSuspend.h>
#include <hardware_legacy/power.h>
#include <utils/Trace.h>
@@ -37,30 +36,7 @@
static std::unordered_map<std::string, sp<IWakeLock>> gWakeLockMap;
static const sp<ISystemSuspend>& getSystemSuspendServiceOnce() {
- using android::system::suspend::V1_0::BpHwSystemSuspend;
- static std::once_flag initFlag;
- static sp<ISystemSuspend> suspendService = nullptr;
-
- // TODO(b/117575503): We use this buffer to make sure that suspendService pointer and the
- // underlying memory are not corrupted before using it. Ideally, memory corruption should be
- // fixed.
- static constexpr size_t bufSize = sizeof(BpHwSystemSuspend);
- static char buf[bufSize];
-
- std::call_once(initFlag, []() {
- // It's possible for the calling process to not have permissions to
- // ISystemSuspend. getService will then return nullptr.
- suspendService = ISystemSuspend::getService();
- if (suspendService) {
- std::memcpy(buf, static_cast<void*>(suspendService.get()), bufSize);
- }
- });
- if (suspendService) {
- if (std::memcmp(buf, static_cast<void*>(suspendService.get()), bufSize) != 0) {
- LOG(FATAL) << "Memory corrupted. Aborting.";
- }
- }
-
+ static sp<ISystemSuspend> suspendService = ISystemSuspend::getService();
return suspendService;
}
diff --git a/power_test.cpp b/power_test.cpp
index bb8e97d..601df64 100644
--- a/power_test.cpp
+++ b/power_test.cpp
@@ -16,19 +16,56 @@
#include <hardware_legacy/power.h>
+#include <csignal>
+#include <cstdlib>
#include <string>
#include <thread>
#include <vector>
#include <gtest/gtest.h>
+using namespace std::chrono_literals;
+
namespace android {
+// Test acquiring/releasing WakeLocks concurrently with process exit.
+TEST(LibpowerTest, ProcessExitTest) {
+ std::atexit([] {
+ // We want to give the other thread enough time trigger a failure and
+ // dump the stack traces.
+ std::this_thread::sleep_for(1s);
+ });
+
+ ASSERT_EXIT(
+ {
+ constexpr int numThreads = 20;
+ std::vector<std::thread> tds;
+ for (int i = 0; i < numThreads; i++) {
+ tds.emplace_back([] {
+ while (true) {
+ // We want ids to be unique.
+ std::string id = std::to_string(rand());
+ ASSERT_EQ(acquire_wake_lock(PARTIAL_WAKE_LOCK, id.c_str()), 0);
+ ASSERT_EQ(release_wake_lock(id.c_str()), 0);
+ }
+ });
+ }
+ for (auto& td : tds) {
+ td.detach();
+ }
+
+ // Give some time for the threads to actually start.
+ std::this_thread::sleep_for(100ms);
+ std::exit(0);
+ },
+ ::testing::ExitedWithCode(0), "");
+}
+
// Stress test acquiring/releasing WakeLocks.
TEST(LibpowerTest, WakeLockStressTest) {
// numThreads threads will acquire/release numLocks locks each.
constexpr int numThreads = 20;
- constexpr int numLocks = 10000;
+ constexpr int numLocks = 1000;
std::vector<std::thread> tds;
for (int i = 0; i < numThreads; i++) {
