Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9e509aba authored by Yifan Hong's avatar Yifan Hong
Browse files

lshal: use std::async 

All of the commands are executed by starting a thread,
and if there is a timeout, sending a SIGINT to the thread,
which invokes pthread_exit from the signal handler. If
pthread_exit is called while the thread is in jemalloc code,
that might cause problems.

Use a standard library std::async function to avoid handling
pthread's manually. This avoids calling pthread_kill(). If
the function times out, simply ignore the future object and
move on. Although this causes memory leak, the lshal tool is
a debugging tool that is intended to run for a short period of
time, not as a daemon. So this is okay.

Test: lshal_test
Bug: 311143089
Change-Id: Id1092fbc3992c62c13c66ddac18105e8dcd6fc81
parent 7b05d26f

cmds/lshal/Timeout.h

+26 −65
Original line number Diff line number Diff line
@@ -16,83 +16,44 @@ 


#pragma once



#include <condition_variable>

#include <chrono>

#include <functional>

#include <mutex>

#include <thread>

#include <future>



#include <hidl/Status.h>

#include <utils/Errors.h>



namespace android {

namespace lshal {



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;

}



// 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.

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;

    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) {



    // 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) {

        return Status::fromStatusT(TIMED_OUT);

    }

    return ret;

    return Status::fromExceptionCode(Status::Exception::EX_ILLEGAL_STATE, "Illegal future_status");

}



} // namespace lshal

} // namespace android

cmds/lshal/main.cpp

+2 −1
Original line number Diff line number Diff line
@@ -18,5 +18,6 @@ 


int main(int argc, char **argv) {

    using namespace ::android::lshal;

    return Lshal{}.main(Arg{argc, argv});

    // Use _exit() to force terminate background threads in Timeout.h

    _exit(Lshal{}.main(Arg{argc, argv}));

}

cmds/lshal/test.cpp

+115 −7
Original line number Diff line number Diff line
@@ -14,6 +14,10 @@ 
 * 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>
@@ -36,6 +40,8 @@ 


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;
@@ -934,10 +940,7 @@ TEST_F(ListTest, DumpDebug) { 
        return hardware::Void();

    }));

    EXPECT_CALL(*serviceManager, get(_, _))

            .WillRepeatedly(

                    Invoke([&](const hidl_string&, const hidl_string& instance) -> sp<IBase> {

                        int id = getIdFromInstanceName(instance);

                        if (id > inheritanceLevel) return nullptr;

            .WillRepeatedly(Invoke([&](const hidl_string&, const hidl_string&) -> sp<IBase> {

                return sp<IBase>(service);

            }));
@@ -957,6 +960,110 @@ 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 {
@@ -1079,5 +1186,6 @@ TEST_F(HelpTest, UnknownOptionHelp2) { 


int main(int argc, char **argv) {

    ::testing::InitGoogleMock(&argc, argv);

    return RUN_ALL_TESTS();

    // Use _exit() to force terminate background threads in Timeout.h

    _exit(RUN_ALL_TESTS());

}