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

Commit ea469023 authored by Shai Barack's avatar Shai Barack
Browse files

Don't call epoll_wait with zero timeout unless watching FDs 

Bug: 410218466
Flag: EXEMPT b/411722849
Test: Looper_test (passes existing tests)
Change-Id: I3972c05c700e8b8e35479e97f39ee5b70b13e7c0
parent 6ec659d3

libutils/Looper.cpp

+13 −6
Original line number Diff line number Diff line
@@ -216,14 +216,18 @@ int Looper::pollInner(int timeoutMillis) { 
    mResponses.clear();

    mResponseIndex = 0;



    struct epoll_event eventItems[EPOLL_MAX_EVENTS];

    int eventCount = 0;

    // Poll if we need to wait or have requests that may have FD events.

    if (timeoutMillis != 0 || mHasRequests) {

        // We are about to idle.

        std::atomic_store_explicit(&mPolling, true, std::memory_order_relaxed);



    struct epoll_event eventItems[EPOLL_MAX_EVENTS];

    int eventCount = epoll_wait(mEpollFd.get(), eventItems, EPOLL_MAX_EVENTS, timeoutMillis);

        eventCount = epoll_wait(mEpollFd.get(), eventItems, EPOLL_MAX_EVENTS, timeoutMillis);



        // No longer idling.

        std::atomic_store_explicit(&mPolling, false, std::memory_order_relaxed);

    }



    // Acquire lock.

    mLock.lock();
@@ -493,6 +497,8 @@ int Looper::addFd(int fd, int ident, int events, const sp<LooperCallback>& callb 
            mRequests.emplace(seq, request);

            seq_it->second = seq;

        }



        mHasRequests = !mRequests.empty();

    } // release lock

    return 1;

}
@@ -560,6 +566,7 @@ int Looper::removeSequenceNumberLocked(SequenceNumber seq) { 
    // updating the epoll set so that we avoid accidentally leaking callbacks.

    mRequests.erase(request_it);

    mSequenceNumberByFd.erase(fd);

    mHasRequests = !mRequests.empty();



    int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_DEL, fd, nullptr);

    if (epollResult < 0) {

libutils/Looper_test.cpp

+3 −6
Original line number Diff line number Diff line
@@ -708,18 +708,15 @@ TEST_F(LooperTest, RemoveMessage_WhenRemovingAllMessagesForHandler_ShouldRemoveT 
    mLooper->sendMessage(handler, Message(MSG_TEST3));

    mLooper->removeMessages(handler);



    StopWatch stopWatch("pollOnce");

    int result = mLooper->pollOnce(0);

    int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime());

    constexpr int MIN_POLL_TIMEOUT_MS = 1;

    int result = mLooper->pollOnce(MIN_POLL_TIMEOUT_MS);



    EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS)

            << "elapsed time should approx. zero because message was sent so looper was awoken";

    EXPECT_EQ(Looper::POLL_WAKE, result)

            << "pollOnce result should be Looper::POLL_WAKE because looper was awoken";

    EXPECT_EQ(size_t(0), handler->messages.size())

            << "no messages to handle";



    result = mLooper->pollOnce(0);

    result = mLooper->pollOnce(MIN_POLL_TIMEOUT_MS);



    EXPECT_EQ(Looper::POLL_TIMEOUT, result)

            << "pollOnce result should be Looper::POLL_TIMEOUT because there was nothing to do";

libutils/abi-dumps/arm64/source-based/libutils.so.lsdump

+262 −256

File changed.

Preview size limit exceeded, changes collapsed.

libutils/abi-dumps/arm_arm64/source-based/libutils.so.lsdump

+256 −250

File changed.

Preview size limit exceeded, changes collapsed.

libutils/include/utils/Looper.h

+9 −0
Original line number Diff line number Diff line
@@ -490,6 +490,15 @@ private: 
    std::unordered_map<SequenceNumber, Request> mRequests;               // guarded by mLock

    std::unordered_map<int /*fd*/, SequenceNumber> mSequenceNumberByFd;  // guarded by mLock



    // Whether mRequests has one or more elements.

    // Volatile to allow the reader (looper thread) to read it while the writer (any thread)

    // may be modifying it without acquiring a lock.

    // This is safe because the looper thread uses this value to determine if it may skip

    // epoll_wait() when there are no requests. If there is a race then it doesn't matter how the

    // race is resolved because the worst case is we'll skip one epoll_wait() call but won't skip

    // subsequent ones.

    volatile bool mHasRequests;



    // The sequence number to use for the next fd that is added to the looper.

    // The sequence number 0 is reserved for the WakeEventFd.

    SequenceNumber mNextRequestSeq;  // guarded by mLock