diff options
| -rw-r--r-- | cmds/lshal/Timeout.h | 91 | ||||
| -rw-r--r-- | cmds/lshal/main.cpp | 3 | ||||
| -rw-r--r-- | cmds/lshal/test.cpp | 122 |
3 files changed, 73 insertions, 143 deletions
diff --git a/cmds/lshal/Timeout.h b/cmds/lshal/Timeout.h index d97ba89122..37f41beea9 100644 --- a/cmds/lshal/Timeout.h +++ b/cmds/lshal/Timeout.h @@ -16,44 +16,83 @@ #pragma once +#include <condition_variable> #include <chrono> -#include <future> +#include <functional> +#include <mutex> +#include <thread> #include <hidl/Status.h> -#include <utils/Errors.h> namespace android { namespace lshal { -// Call function on interfaceObject and wait for result until the given timeout has reached. -// Callback functions pass to timeoutIPC() may be executed after the this function -// has returned, especially if deadline has been reached. Hence, care must be taken when passing -// data between the background thread and the main thread. See b/311143089. +class BackgroundTaskState { +public: + explicit BackgroundTaskState(std::function<void(void)> &&func) + : mFunc(std::forward<decltype(func)>(func)) {} + void notify() { + std::unique_lock<std::mutex> lock(mMutex); + mFinished = true; + lock.unlock(); + mCondVar.notify_all(); + } + template<class C, class D> + bool wait(std::chrono::time_point<C, D> end) { + std::unique_lock<std::mutex> lock(mMutex); + mCondVar.wait_until(lock, end, [this](){ return this->mFinished; }); + return mFinished; + } + void operator()() { + mFunc(); + } +private: + std::mutex mMutex; + std::condition_variable mCondVar; + bool mFinished = false; + std::function<void(void)> mFunc; +}; + +void *callAndNotify(void *data) { + BackgroundTaskState &state = *static_cast<BackgroundTaskState *>(data); + state(); + state.notify(); + return nullptr; +} + +template<class R, class P> +bool timeout(std::chrono::duration<R, P> delay, std::function<void(void)> &&func) { + auto now = std::chrono::system_clock::now(); + BackgroundTaskState state{std::forward<decltype(func)>(func)}; + pthread_t thread; + if (pthread_create(&thread, nullptr, callAndNotify, &state)) { + std::cerr << "FATAL: could not create background thread." << std::endl; + return false; + } + bool success = state.wait(now + delay); + if (!success) { + pthread_kill(thread, SIGINT); + } + pthread_join(thread, nullptr); + return success; +} + template<class R, class P, class Function, class I, class... Args> typename std::invoke_result<Function, I *, Args...>::type timeoutIPC(std::chrono::duration<R, P> wait, const sp<I> &interfaceObject, Function &&func, Args &&... args) { using ::android::hardware::Status; - - // Execute on a background thread but do not defer execution. - auto future = - std::async(std::launch::async, func, interfaceObject, std::forward<Args>(args)...); - auto status = future.wait_for(wait); - if (status == std::future_status::ready) { - return future.get(); - } - - // This future belongs to a background thread that we no longer care about. - // Putting this in the global list avoids std::future::~future() that may wait for the - // result to come back. - // This leaks memory, but lshal is a debugging tool, so this is fine. - static std::vector<decltype(future)> gDeadPool{}; - gDeadPool.emplace_back(std::move(future)); - - if (status == std::future_status::timeout) { + typename std::result_of<Function(I *, Args...)>::type ret{Status::ok()}; + auto boundFunc = std::bind(std::forward<Function>(func), + interfaceObject.get(), std::forward<Args>(args)...); + bool success = timeout(wait, [&ret, &boundFunc] { + ret = std::move(boundFunc()); + }); + if (!success) { return Status::fromStatusT(TIMED_OUT); } - return Status::fromExceptionCode(Status::Exception::EX_ILLEGAL_STATE, "Illegal future_status"); + return ret; } -} // namespace lshal -} // namespace android + +} // namespace lshal +} // namespace android diff --git a/cmds/lshal/main.cpp b/cmds/lshal/main.cpp index bd5fa32521..366c9383a2 100644 --- a/cmds/lshal/main.cpp +++ b/cmds/lshal/main.cpp @@ -18,6 +18,5 @@ int main(int argc, char **argv) { using namespace ::android::lshal; - // Use _exit() to force terminate background threads in Timeout.h - _exit(Lshal{}.main(Arg{argc, argv})); + return Lshal{}.main(Arg{argc, argv}); } diff --git a/cmds/lshal/test.cpp b/cmds/lshal/test.cpp index c24f827e73..cba7c4bf2a 100644 --- a/cmds/lshal/test.cpp +++ b/cmds/lshal/test.cpp @@ -14,10 +14,6 @@ * limitations under the License. */ -#include <chrono> -#include <future> -#include <mutex> -#include "android/hidl/base/1.0/IBase.h" #define LOG_TAG "Lshal" #include <android-base/logging.h> @@ -40,8 +36,6 @@ using namespace testing; -using std::chrono_literals::operator""ms; - using ::android::hidl::base::V1_0::DebugInfo; using ::android::hidl::base::V1_0::IBase; using ::android::hidl::manager::V1_0::IServiceManager; @@ -940,9 +934,12 @@ TEST_F(ListTest, DumpDebug) { return hardware::Void(); })); EXPECT_CALL(*serviceManager, get(_, _)) - .WillRepeatedly(Invoke([&](const hidl_string&, const hidl_string&) -> sp<IBase> { - return sp<IBase>(service); - })); + .WillRepeatedly( + Invoke([&](const hidl_string&, const hidl_string& instance) -> sp<IBase> { + int id = getIdFromInstanceName(instance); + if (id > inheritanceLevel) return nullptr; + return sp<IBase>(service); + })); const std::string expected = "[fake description 0]\n" "Interface\n" @@ -960,110 +957,6 @@ TEST_F(ListTest, DumpDebug) { EXPECT_EQ("", err.str()); } -// In SlowService, everything goes slooooooow. Each IPC call will wait for -// the specified time before calling the callback function or returning. -class SlowService : public IBase { -public: - explicit SlowService(std::chrono::milliseconds wait) : mWait(wait) {} - android::hardware::Return<void> interfaceDescriptor(interfaceDescriptor_cb cb) override { - std::this_thread::sleep_for(mWait); - cb(getInterfaceName(1)); - storeHistory("interfaceDescriptor"); - return hardware::Void(); - } - android::hardware::Return<void> interfaceChain(interfaceChain_cb cb) override { - std::this_thread::sleep_for(mWait); - std::vector<hidl_string> ret; - ret.push_back(getInterfaceName(1)); - ret.push_back(IBase::descriptor); - cb(ret); - storeHistory("interfaceChain"); - return hardware::Void(); - } - android::hardware::Return<void> getHashChain(getHashChain_cb cb) override { - std::this_thread::sleep_for(mWait); - std::vector<hidl_hash> ret; - ret.push_back(getHashFromId(0)); - ret.push_back(getHashFromId(0xff)); - cb(ret); - storeHistory("getHashChain"); - return hardware::Void(); - } - android::hardware::Return<void> debug(const hidl_handle&, - const hidl_vec<hidl_string>&) override { - std::this_thread::sleep_for(mWait); - storeHistory("debug"); - return Void(); - } - - template <class R, class P, class Pred> - bool waitForHistory(std::chrono::duration<R, P> wait, Pred predicate) { - std::unique_lock<std::mutex> lock(mLock); - return mCv.wait_for(lock, wait, [&]() { return predicate(mCallHistory); }); - } - -private: - void storeHistory(std::string hist) { - { - std::lock_guard<std::mutex> lock(mLock); - mCallHistory.emplace_back(std::move(hist)); - } - mCv.notify_all(); - } - - const std::chrono::milliseconds mWait; - std::mutex mLock; - std::condition_variable mCv; - // List of functions that have finished being called on this interface. - std::vector<std::string> mCallHistory; -}; - -class TimeoutTest : public ListTest { -public: - void setMockServiceManager(sp<IBase> service) { - EXPECT_CALL(*serviceManager, list(_)) - .WillRepeatedly(Invoke([&](IServiceManager::list_cb cb) { - std::vector<hidl_string> ret; - ret.push_back(getInterfaceName(1) + "/default"); - cb(ret); - return hardware::Void(); - })); - EXPECT_CALL(*serviceManager, get(_, _)) - .WillRepeatedly(Invoke([&](const hidl_string&, const hidl_string&) -> sp<IBase> { - return service; - })); - } -}; - -TEST_F(TimeoutTest, BackgroundThreadIsKept) { - auto lshalIpcTimeout = 100ms; - auto serviceIpcTimeout = 200ms; - lshal->setWaitTimeForTest(lshalIpcTimeout, lshalIpcTimeout); - sp<SlowService> service = new SlowService(serviceIpcTimeout); - setMockServiceManager(service); - - optind = 1; // mimic Lshal::parseArg() - EXPECT_NE(0u, mockList->main(createArg({"lshal", "--types=b", "-i", "--neat"}))); - EXPECT_THAT(err.str(), HasSubstr("Skipping \"a.h.foo1@1.0::IFoo/default\"")); - EXPECT_TRUE(service->waitForHistory(serviceIpcTimeout * 5, [](const auto& hist) { - return hist.size() == 1 && hist[0] == "interfaceChain"; - })) << "The background thread should continue after the main thread moves on, but it is killed"; -} - -TEST_F(TimeoutTest, BackgroundThreadDoesNotBlockMainThread) { - auto lshalIpcTimeout = 100ms; - auto serviceIpcTimeout = 2000ms; - auto start = std::chrono::system_clock::now(); - lshal->setWaitTimeForTest(lshalIpcTimeout, lshalIpcTimeout); - sp<SlowService> service = new SlowService(serviceIpcTimeout); - setMockServiceManager(service); - - optind = 1; // mimic Lshal::parseArg() - EXPECT_NE(0u, mockList->main(createArg({"lshal", "--types=b", "-i", "--neat"}))); - EXPECT_LE(std::chrono::system_clock::now(), start + 5 * lshalIpcTimeout) - << "The main thread should not be blocked by the background task"; -} - class ListVintfTest : public ListTest { public: virtual void SetUp() override { @@ -1186,6 +1079,5 @@ TEST_F(HelpTest, UnknownOptionHelp2) { int main(int argc, char **argv) { ::testing::InitGoogleMock(&argc, argv); - // Use _exit() to force terminate background threads in Timeout.h - _exit(RUN_ALL_TESTS()); + return RUN_ALL_TESTS(); } |