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

Commit 42b99304 authored by Andy Hung's avatar Andy Hung
Browse files

MediaMetrics:LockWrap: Use recursive mutex. 

Because one might reference the object twice when
the end of the full expression goes to ;

Test: atest mediametrics_tests
Bug: 138583596
Change-Id: I80ced3af615578d63310acaf70142be900855902
parent bc09c479

services/mediametrics/Wrap.h

+27 −12
Original line number Diff line number Diff line
@@ -150,11 +150,17 @@ class LockWrap { 
      */

    class LockedPointer {

        friend LockWrap;

        LockedPointer(T *t, std::mutex *lock)

            : mT(t), mLock(*lock) {}

        LockedPointer(T *t, std::recursive_mutex *lock, std::atomic<size_t> *recursionDepth)

            : mT(t), mLock(*lock), mRecursionDepth(recursionDepth) { ++*mRecursionDepth; }



        T* const mT;

        std::lock_guard<std::mutex> mLock;

        std::lock_guard<std::recursive_mutex> mLock;

        std::atomic<size_t>* mRecursionDepth;

    public:

        ~LockedPointer() {

            --*mRecursionDepth; // Used for testing, we do not check underflow.

        }



        const T* operator->() const {

            return mT;

        }
@@ -163,27 +169,36 @@ class LockWrap { 
        }

    };



    mutable std::mutex mLock;

    // We must use a recursive mutex because the end of the full expression may

    // involve another reference to T->.

    //

    // A recursive mutex allows the same thread to recursively acquire,

    // but different thread would block.

    //

    // Example which fails with a normal mutex:

    //

    // android::mediametrics::LockWrap<std::vector<int>> v{std::initializer_list<int>{1, 2}};

    // const int sum = v->operator[](0) + v->operator[](1);

    //

    mutable std::recursive_mutex mLock;

    mutable T mT;

    mutable std::atomic<size_t> mRecursionDepth{};  // Used for testing.



public:

    template <typename... Args>

    explicit LockWrap(Args&&... args) : mT(std::forward<Args>(args)...) {}



    const LockedPointer operator->() const {

        return LockedPointer(&mT, &mLock);

        return LockedPointer(&mT, &mLock, &mRecursionDepth);

    }

    LockedPointer operator->() {

        return LockedPointer(&mT, &mLock);

        return LockedPointer(&mT, &mLock, &mRecursionDepth);

    }



    // Returns the lock depth of the recursive mutex.

    // @TestApi

    bool isLocked() const {

        if (mLock.try_lock()) {

            mLock.unlock();

            return false; // we were able to get the lock.

        }

        return true; // we were NOT able to get the lock.

    size_t getRecursionDepth() const {

        return mRecursionDepth;

    }

};

services/mediametrics/tests/mediametrics_tests.cpp

+24 −1
Original line number Diff line number Diff line
@@ -112,6 +112,27 @@ TEST(mediametrics_tests, lock_wrap) { 
  s3->operator=("abc");

  ASSERT_EQ('b', s3->operator[](1)); // s2[1] = 'b';



  // Check that we can recursively hold lock.

  android::mediametrics::LockWrap<std::vector<int>> v{std::initializer_list<int>{1, 2}};

  v->push_back(3);

  v->push_back(4);

  ASSERT_EQ(1, v->operator[](0));

  ASSERT_EQ(2, v->operator[](1));

  ASSERT_EQ(3, v->operator[](2));

  ASSERT_EQ(4, v->operator[](3));

  // The end of the full expression here requires recursive depth of 4.

  ASSERT_EQ(10, v->operator[](0) + v->operator[](1) + v->operator[](2) + v->operator[](3));



  // Mikhail's note: a non-recursive lock implementation could be used if one obtains

  // the LockedPointer helper object like this and directly hold the lock through RAII,

  // though it is trickier in use.

  //

  // We include an example here for completeness.

  {

    auto l = v.operator->();

    ASSERT_EQ(10, l->operator[](0) + l->operator[](1) + l->operator[](2) + l->operator[](3));

  }



  // Use Thunk to check whether we have the lock when calling a method through LockWrap.



  class Thunk {
@@ -123,11 +144,13 @@ TEST(mediametrics_tests, lock_wrap) { 
  };



  android::mediametrics::LockWrap<Thunk> s4([&]{

    ASSERT_EQ(true, s4.isLocked()); // we must be locked when thunk() is called.

    ASSERT_EQ((size_t)1, s4.getRecursionDepth()); // we must be locked when thunk() is called.

  });



  ASSERT_EQ((size_t)0, s4.getRecursionDepth());

  // This will fail if we are not locked during method access.

  s4->thunk();

  ASSERT_EQ((size_t)0, s4.getRecursionDepth());

}



TEST(mediametrics_tests, lock_wrap_multithread) {