diff options
author | Kalesh Singh <kaleshsingh@google.com> | 2021-02-22 12:59:17 -0500 |
---|---|---|
committer | Kalesh Singh <kaleshsingh@google.com> | 2021-02-23 10:06:19 -0500 |
commit | d8da2af1e570b73f228cdee7305b83a078a791bf (patch) | |
tree | 042e7749fea08984cc7957f3bb613b6c54625bcb | |
parent | 1da7880c265f064e48dd2c03566ce1b123ecfd84 (diff) | |
download | libhardware_legacy-d8da2af1e570b73f228cdee7305b83a078a791bf.tar.gz |
RAII style wakelocks: Add tryGet() factory method.
Wakelock acquisition can fail if suspend service is not available.
Make this clear by returning an optional value that client must
check before entering wakelock critical sections.
Bug: b/179229598
Test: Boot test on Pixel 4 device
Change-Id: Ied00fa919c20d8a30f1c40ee34a91a2a7e240689
-rw-r--r-- | block_suspend.cpp | 6 | ||||
-rw-r--r-- | include/wakelock/wakelock.h | 13 | ||||
-rw-r--r-- | power.cpp | 29 | ||||
-rw-r--r-- | power_test.cpp | 5 |
4 files changed, 45 insertions, 8 deletions
diff --git a/block_suspend.cpp b/block_suspend.cpp index 18c93d4..d780904 100644 --- a/block_suspend.cpp +++ b/block_suspend.cpp @@ -34,7 +34,11 @@ int main(int argc, char ** /* argv */) { return 0; } - android::wakelock::WakeLock wl{gWakeLockName}; // RAII object + auto wl = android::wakelock::WakeLock::tryGet(gWakeLockName); // RAII object + if (!wl.has_value()) { + return EXIT_FAILURE; + } + while (true) { sleep(1000000); }; std::abort(); // should never reach here return 0; diff --git a/include/wakelock/wakelock.h b/include/wakelock/wakelock.h index 75a7c72..44c962c 100644 --- a/include/wakelock/wakelock.h +++ b/include/wakelock/wakelock.h @@ -17,6 +17,7 @@ #pragma once #include <memory> +#include <optional> #include <string> namespace android { @@ -24,13 +25,17 @@ namespace wakelock { // RAII-style wake lock implementation class WakeLock { - public: - WakeLock(const std::string& name); - ~WakeLock(); - private: class WakeLockImpl; std::unique_ptr<WakeLockImpl> mImpl; + + public: + static std::optional<WakeLock> tryGet(const std::string& name); + // Constructor is only made public for use with std::optional. + // It is not intended to be and cannot be invoked from public context, + // since private WakeLockImpl prevents calling the constructor directly. + WakeLock(std::unique_ptr<WakeLockImpl> wlImpl); + ~WakeLock(); }; } // namespace wakelock @@ -88,26 +88,51 @@ class WakeLock::WakeLockImpl { public: WakeLockImpl(const std::string& name); ~WakeLockImpl(); + bool acquireOk(); private: sp<IWakeLock> mWakeLock; }; -WakeLock::WakeLock(const std::string& name) : mImpl(std::make_unique<WakeLockImpl>(name)) {} +std::optional<WakeLock> WakeLock::tryGet(const std::string& name) { + std::unique_ptr<WakeLockImpl> wlImpl = std::make_unique<WakeLockImpl>(name); + if (wlImpl->acquireOk()) { + return { std::move(wlImpl) }; + } else { + LOG(ERROR) << "Failed to acquire wakelock: " << name; + return {}; + } +} + +WakeLock::WakeLock(std::unique_ptr<WakeLockImpl> wlImpl) : mImpl(std::move(wlImpl)) {} WakeLock::~WakeLock() = default; WakeLock::WakeLockImpl::WakeLockImpl(const std::string& name) : mWakeLock(nullptr) { static sp<ISystemSuspend> suspendService = ISystemSuspend::getService(); - mWakeLock = suspendService->acquireWakeLock(WakeLockType::PARTIAL, name); + auto ret = suspendService->acquireWakeLock(WakeLockType::PARTIAL, name); + // It's possible that during device SystemSuspend service is not avaiable. In these + // situations HIDL calls to it will result in a DEAD_OBJECT transaction error. + if (ret.isDeadObject()) { + LOG(ERROR) << "ISuspendService::acquireWakeLock() call failed: " << ret.description(); + } else { + mWakeLock = ret; + } } WakeLock::WakeLockImpl::~WakeLockImpl() { + if (!acquireOk()) { + return; + } auto ret = mWakeLock->release(); if (!ret.isOk()) { LOG(ERROR) << "IWakeLock::release() call failed: " << ret.description(); } } +bool WakeLock::WakeLockImpl::acquireOk() { + return mWakeLock != nullptr; +} + } // namespace wakelock } // namespace android diff --git a/power_test.cpp b/power_test.cpp index 32cff70..9a68651 100644 --- a/power_test.cpp +++ b/power_test.cpp @@ -121,7 +121,10 @@ class WakeLockTest : public ::testing::Test { TEST_F(WakeLockTest, WakeLockDestructor) { auto name = std::to_string(rand()); { - android::wakelock::WakeLock wl{name}; + auto wl = android::wakelock::WakeLock::tryGet(name); + if (!wl.has_value()) { + return; + } WakeLockInfo info; auto success = findWakeLockInfoByName(controlService, name, &info); |