Revert "lshal: use std::async"
This reverts commit 9e509aba9efba4de1253acd332745521d571a474.
Reason for revert: broke adb shell lshal debug
Bug: 323268003
Change-Id: I005ae901dc347a0f4d3ad617ab148c8023786e7b
diff --git a/cmds/lshal/Timeout.h b/cmds/lshal/Timeout.h
index 59ca3bf..805e8dc 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::result_of<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 bd5fa32..366c938 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 c24f827..cba7c4b 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 @@
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 @@
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 @@
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();
}