Merge changes If6507a05,Ie35de689 into tm-qpr-dev am: 1c7fb94296 (5d61c8cc) · Commits · e / os / android_frameworks_av

media/utils/TimeCheck.cpp

+13 −9

Original line number	Original line	Diff line number	Diff line
	@@ -139,12 +139,13 @@ std::string TimeCheck::toString() {
	return getTimeCheckThread().toString();		return getTimeCheckThread().toString();
	}		}

	TimeCheck::TimeCheck(std::string_view tag, OnTimerFunc&& onTimer, uint32_t requestedTimeoutMs,		TimeCheck::TimeCheck(std::string_view tag, OnTimerFunc&& onTimer, Duration requestedTimeoutDuration,
	bool crashOnTimeout)		Duration secondChanceDuration, bool crashOnTimeout)
	: mTimeCheckHandler{ std::make_shared<TimeCheckHandler>(		: mTimeCheckHandler{ std::make_shared<TimeCheckHandler>(
	tag, std::move(onTimer), crashOnTimeout, std::chrono::milliseconds(requestedTimeoutMs),		tag, std::move(onTimer), crashOnTimeout, requestedTimeoutDuration,
	std::chrono::system_clock::now(), gettid()) }		secondChanceDuration, std::chrono::system_clock::now(), gettid()) }
	, mTimerHandle(requestedTimeoutMs == 0		, mTimerHandle(requestedTimeoutDuration.count() == 0
			/* for TimeCheck we don't consider a non-zero secondChanceDuration here */
	? getTimeCheckThread().trackTask(mTimeCheckHandler->tag)		? getTimeCheckThread().trackTask(mTimeCheckHandler->tag)
	: getTimeCheckThread().scheduleTask(		: getTimeCheckThread().scheduleTask(
	mTimeCheckHandler->tag,		mTimeCheckHandler->tag,
	@@ -154,7 +155,8 @@ TimeCheck::TimeCheck(std::string_view tag, OnTimerFunc&& onTimer, uint32_t reque
	[ timeCheckHandler = mTimeCheckHandler ](TimerThread::Handle timerHandle) {		[ timeCheckHandler = mTimeCheckHandler ](TimerThread::Handle timerHandle) {
	timeCheckHandler->onTimeout(timerHandle);		timeCheckHandler->onTimeout(timerHandle);
	},		},
	std::chrono::milliseconds(requestedTimeoutMs))) {}		requestedTimeoutDuration,
			secondChanceDuration)) {}

	TimeCheck::~TimeCheck() {		TimeCheck::~TimeCheck() {
	if (mTimeCheckHandler) {		if (mTimeCheckHandler) {
	@@ -228,6 +230,8 @@ void TimeCheck::TimeCheckHandler::onTimeout(TimerThread::Handle timerHandle) con
	endSystemTime - startSystemTime).count();		endSystemTime - startSystemTime).count();
	const float requestedTimeoutMs = std::chrono::duration_cast<FloatMs>(		const float requestedTimeoutMs = std::chrono::duration_cast<FloatMs>(
	timeoutDuration).count();		timeoutDuration).count();
			const float secondChanceMs = std::chrono::duration_cast<FloatMs>(
			secondChanceDuration).count();

	if (onTimer) {		if (onTimer) {
	onTimer(true /* timeout */, elapsedSteadyMs);		onTimer(true /* timeout */, elapsedSteadyMs);
	@@ -262,8 +266,8 @@ void TimeCheck::TimeCheckHandler::onTimeout(TimerThread::Handle timerHandle) con
	.append(tag)		.append(tag)
	.append(" scheduled ").append(formatTime(startSystemTime))		.append(" scheduled ").append(formatTime(startSystemTime))
	.append(" on thread ").append(std::to_string(tid)).append("\n")		.append(" on thread ").append(std::to_string(tid)).append("\n")
	.append(analyzeTimeouts(		.append(analyzeTimeouts(requestedTimeoutMs + secondChanceMs,
	requestedTimeoutMs, elapsedSteadyMs, elapsedSystemMs)).append("\n")		elapsedSteadyMs, elapsedSystemMs)).append("\n")
	.append(halPids).append("\n")		.append(halPids).append("\n")
	.append(summary);		.append(summary);

	@@ -295,7 +299,7 @@ mediautils::TimeCheck makeTimeCheckStatsForClassMethod(
	} else {		} else {
	stats->event(safeMethodName.asStringView(), elapsedMs);		stats->event(safeMethodName.asStringView(), elapsedMs);
	}		}
	}, 0 /* requestedTimeoutMs */);		}, {} /* timeoutDuration /, {} / secondChanceDuration /, false / crashOnTimeout */);
	}		}

	} // namespace android::mediautils		} // namespace android::mediautils

media/utils/TimerThread-test.cpp

+85 −61

Original line number	Original line	Diff line number	Diff line
	@@ -33,12 +33,27 @@ inline size_t countChars(std::string_view s, char c) {
	return std::count(s.begin(), s.end(), c);		return std::count(s.begin(), s.end(), c);
	}		}

	TEST(TimerThread, Basic) {
			// Split msec time between timeout and second chance time
			// This tests expiration times weighted between timeout and the second chance time.
			#define DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(msec, frac) \
			std::chrono::milliseconds(int((msec) * (frac)) + 1), \
			std::chrono::milliseconds(int((msec) * (1.f - (frac))))

			// The TimerThreadTest is parameterized on a fraction between 0.f and 1.f which
			// is how the total timeout time is split between the first timeout and the second chance time.
			//
			class TimerThreadTest : public ::testing::TestWithParam<float> {
			protected:

			static void testBasic() {
			const auto frac = GetParam();

	std::atomic<bool> taskRan = false;		std::atomic<bool> taskRan = false;
	TimerThread thread;		TimerThread thread;
	TimerThread::Handle handle =		TimerThread::Handle handle =
	thread.scheduleTask("Basic", [&taskRan](TimerThread::Handle handle __unused) {		thread.scheduleTask("Basic", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan = true; }, 100ms);		taskRan = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(100, frac));
	ASSERT_TRUE(TimerThread::isTimeoutHandle(handle));		ASSERT_TRUE(TimerThread::isTimeoutHandle(handle));
	std::this_thread::sleep_for(100ms - kJitter);		std::this_thread::sleep_for(100ms - kJitter);
	ASSERT_FALSE(taskRan);		ASSERT_FALSE(taskRan);
	@@ -47,12 +62,14 @@ TEST(TimerThread, Basic) {
	ASSERT_EQ(1, countChars(thread.retiredToString(), REQUEST_START));		ASSERT_EQ(1, countChars(thread.retiredToString(), REQUEST_START));
	}		}

	TEST(TimerThread, Cancel) {		static void testCancel() {
			const auto frac = GetParam();

	std::atomic<bool> taskRan = false;		std::atomic<bool> taskRan = false;
	TimerThread thread;		TimerThread thread;
	TimerThread::Handle handle =		TimerThread::Handle handle =
	thread.scheduleTask("Cancel", [&taskRan](TimerThread::Handle handle __unused) {		thread.scheduleTask("Cancel", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan = true; }, 100ms);		taskRan = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(100, frac));
	ASSERT_TRUE(TimerThread::isTimeoutHandle(handle));		ASSERT_TRUE(TimerThread::isTimeoutHandle(handle));
	std::this_thread::sleep_for(100ms - kJitter);		std::this_thread::sleep_for(100ms - kJitter);
	ASSERT_FALSE(taskRan);		ASSERT_FALSE(taskRan);
	@@ -62,13 +79,16 @@ TEST(TimerThread, Cancel) {
	ASSERT_EQ(1, countChars(thread.retiredToString(), REQUEST_START));		ASSERT_EQ(1, countChars(thread.retiredToString(), REQUEST_START));
	}		}

	TEST(TimerThread, CancelAfterRun) {		static void testCancelAfterRun() {
			const auto frac = GetParam();

	std::atomic<bool> taskRan = false;		std::atomic<bool> taskRan = false;
	TimerThread thread;		TimerThread thread;
	TimerThread::Handle handle =		TimerThread::Handle handle =
	thread.scheduleTask("CancelAfterRun",		thread.scheduleTask("CancelAfterRun",
	[&taskRan](TimerThread::Handle handle __unused) {		[&taskRan](TimerThread::Handle handle __unused) {
	taskRan = true; }, 100ms);		taskRan = true; },
			DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(100, frac));
	ASSERT_TRUE(TimerThread::isTimeoutHandle(handle));		ASSERT_TRUE(TimerThread::isTimeoutHandle(handle));
	std::this_thread::sleep_for(100ms + kJitter);		std::this_thread::sleep_for(100ms + kJitter);
	ASSERT_TRUE(taskRan);		ASSERT_TRUE(taskRan);
	@@ -76,83 +96,70 @@ TEST(TimerThread, CancelAfterRun) {
	ASSERT_EQ(1, countChars(thread.retiredToString(), REQUEST_START));		ASSERT_EQ(1, countChars(thread.retiredToString(), REQUEST_START));
	}		}

	TEST(TimerThread, MultipleTasks) {		static void testMultipleTasks() {
			const auto frac = GetParam();

	std::array<std::atomic<bool>, 6> taskRan{};		std::array<std::atomic<bool>, 6> taskRan{};
	TimerThread thread;		TimerThread thread;

	auto startTime = std::chrono::steady_clock::now();		auto startTime = std::chrono::steady_clock::now();

	thread.scheduleTask("0", [&taskRan](TimerThread::Handle handle __unused) {		thread.scheduleTask("0", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan[0] = true; }, 300ms);		taskRan[0] = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(300, frac));
	thread.scheduleTask("1", [&taskRan](TimerThread::Handle handle __unused) {		thread.scheduleTask("1", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan[1] = true; }, 100ms);		taskRan[1] = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(100, frac));
	thread.scheduleTask("2", [&taskRan](TimerThread::Handle handle __unused) {		thread.scheduleTask("2", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan[2] = true; }, 200ms);		taskRan[2] = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(200, frac));
	thread.scheduleTask("3", [&taskRan](TimerThread::Handle handle __unused) {		thread.scheduleTask("3", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan[3] = true; }, 400ms);		taskRan[3] = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(400, frac));
	auto handle4 = thread.scheduleTask("4", [&taskRan](TimerThread::Handle handle __unused) {		auto handle4 = thread.scheduleTask("4", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan[4] = true; }, 200ms);		taskRan[4] = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(200, frac));
	thread.scheduleTask("5", [&taskRan](TimerThread::Handle handle __unused) {		thread.scheduleTask("5", [&taskRan](TimerThread::Handle handle __unused) {
	taskRan[5] = true; }, 200ms);		taskRan[5] = true; }, DISTRIBUTE_TIMEOUT_SECONDCHANCE_MS_FRAC(200, frac));

	// 6 tasks pending		// 6 tasks pending
	ASSERT_EQ(6, countChars(thread.pendingToString(), REQUEST_START));		ASSERT_EQ(6, countChars(thread.pendingToString(), REQUEST_START));
	// 0 tasks completed		// 0 tasks completed
	ASSERT_EQ(0, countChars(thread.retiredToString(), REQUEST_START));		ASSERT_EQ(0, countChars(thread.retiredToString(), REQUEST_START));

			// None of the tasks are expected to have finished at the start.
			std::array<std::atomic<bool>, 6> expected{};

	// Task 1 should trigger around 100ms.		// Task 1 should trigger around 100ms.
	std::this_thread::sleep_until(startTime + 100ms - kJitter);		std::this_thread::sleep_until(startTime + 100ms - kJitter);
	ASSERT_FALSE(taskRan[0]);
	ASSERT_FALSE(taskRan[1]);		ASSERT_EQ(expected, taskRan);
	ASSERT_FALSE(taskRan[2]);
	ASSERT_FALSE(taskRan[3]);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_FALSE(taskRan[5]);

	std::this_thread::sleep_until(startTime + 100ms + kJitter);		std::this_thread::sleep_until(startTime + 100ms + kJitter);
	ASSERT_FALSE(taskRan[0]);
	ASSERT_TRUE(taskRan[1]);		expected[1] = true;
	ASSERT_FALSE(taskRan[2]);		ASSERT_EQ(expected, taskRan);
	ASSERT_FALSE(taskRan[3]);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_FALSE(taskRan[5]);

	// Cancel task 4 before it gets a chance to run.		// Cancel task 4 before it gets a chance to run.
	thread.cancelTask(handle4);		thread.cancelTask(handle4);

	// Tasks 2 and 5 should trigger around 200ms.		// Tasks 2 and 5 should trigger around 200ms.
	std::this_thread::sleep_until(startTime + 200ms - kJitter);		std::this_thread::sleep_until(startTime + 200ms - kJitter);
	ASSERT_FALSE(taskRan[0]);
	ASSERT_TRUE(taskRan[1]);		ASSERT_EQ(expected, taskRan);
	ASSERT_FALSE(taskRan[2]);
	ASSERT_FALSE(taskRan[3]);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_FALSE(taskRan[5]);

	std::this_thread::sleep_until(startTime + 200ms + kJitter);		std::this_thread::sleep_until(startTime + 200ms + kJitter);
	ASSERT_FALSE(taskRan[0]);
	ASSERT_TRUE(taskRan[1]);		expected[2] = true;
	ASSERT_TRUE(taskRan[2]);		expected[5] = true;
	ASSERT_FALSE(taskRan[3]);		ASSERT_EQ(expected, taskRan);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_TRUE(taskRan[5]);

	// Task 0 should trigger around 300ms.		// Task 0 should trigger around 300ms.
	std::this_thread::sleep_until(startTime + 300ms - kJitter);		std::this_thread::sleep_until(startTime + 300ms - kJitter);
	ASSERT_FALSE(taskRan[0]);
	ASSERT_TRUE(taskRan[1]);		ASSERT_EQ(expected, taskRan);
	ASSERT_TRUE(taskRan[2]);
	ASSERT_FALSE(taskRan[3]);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_TRUE(taskRan[5]);

	std::this_thread::sleep_until(startTime + 300ms + kJitter);		std::this_thread::sleep_until(startTime + 300ms + kJitter);
	ASSERT_TRUE(taskRan[0]);
	ASSERT_TRUE(taskRan[1]);		expected[0] = true;
	ASSERT_TRUE(taskRan[2]);		ASSERT_EQ(expected, taskRan);
	ASSERT_FALSE(taskRan[3]);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_TRUE(taskRan[5]);

	// 1 task pending		// 1 task pending
	ASSERT_EQ(1, countChars(thread.pendingToString(), REQUEST_START));		ASSERT_EQ(1, countChars(thread.pendingToString(), REQUEST_START));
	@@ -161,23 +168,16 @@ TEST(TimerThread, MultipleTasks) {

	// Task 3 should trigger around 400ms.		// Task 3 should trigger around 400ms.
	std::this_thread::sleep_until(startTime + 400ms - kJitter);		std::this_thread::sleep_until(startTime + 400ms - kJitter);
	ASSERT_TRUE(taskRan[0]);
	ASSERT_TRUE(taskRan[1]);		ASSERT_EQ(expected, taskRan);
	ASSERT_TRUE(taskRan[2]);
	ASSERT_FALSE(taskRan[3]);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_TRUE(taskRan[5]);

	// 4 tasks ran and 1 cancelled		// 4 tasks ran and 1 cancelled
	ASSERT_EQ(4 + 1, countChars(thread.retiredToString(), REQUEST_START));		ASSERT_EQ(4 + 1, countChars(thread.retiredToString(), REQUEST_START));

	std::this_thread::sleep_until(startTime + 400ms + kJitter);		std::this_thread::sleep_until(startTime + 400ms + kJitter);
	ASSERT_TRUE(taskRan[0]);
	ASSERT_TRUE(taskRan[1]);		expected[3] = true;
	ASSERT_TRUE(taskRan[2]);		ASSERT_EQ(expected, taskRan);
	ASSERT_TRUE(taskRan[3]);
	ASSERT_FALSE(taskRan[4]);
	ASSERT_TRUE(taskRan[5]);

	// 0 tasks pending		// 0 tasks pending
	ASSERT_EQ(0, countChars(thread.pendingToString(), REQUEST_START));		ASSERT_EQ(0, countChars(thread.pendingToString(), REQUEST_START));
	@@ -185,6 +185,30 @@ TEST(TimerThread, MultipleTasks) {
	ASSERT_EQ(5 + 1, countChars(thread.retiredToString(), REQUEST_START));		ASSERT_EQ(5 + 1, countChars(thread.retiredToString(), REQUEST_START));
	}		}

			}; // class TimerThreadTest

			TEST_P(TimerThreadTest, Basic) {
			testBasic();
			}

			TEST_P(TimerThreadTest, Cancel) {
			testCancel();
			}

			TEST_P(TimerThreadTest, CancelAfterRun) {
			testCancelAfterRun();
			}

			TEST_P(TimerThreadTest, MultipleTasks) {
			testMultipleTasks();
			}

			INSTANTIATE_TEST_CASE_P(
			TimerThread,
			TimerThreadTest,
			::testing::Values(0.f, 0.5f, 1.f)
			);

	TEST(TimerThread, TrackedTasks) {		TEST(TimerThread, TrackedTasks) {
	TimerThread thread;		TimerThread thread;

media/utils/TimerThread.cpp

+94 −16

Original line number	Original line	Diff line number	Diff line
	@@ -23,6 +23,7 @@

	#include <mediautils/MediaUtilsDelayed.h>		#include <mediautils/MediaUtilsDelayed.h>
	#include <mediautils/TimerThread.h>		#include <mediautils/TimerThread.h>
			#include <utils/Log.h>
	#include <utils/ThreadDefs.h>		#include <utils/ThreadDefs.h>

	using namespace std::chrono_literals;		using namespace std::chrono_literals;
	@@ -33,17 +34,19 @@ extern std::string formatTime(std::chrono::system_clock::time_point t);
	extern std::string_view timeSuffix(std::string_view time1, std::string_view time2);		extern std::string_view timeSuffix(std::string_view time1, std::string_view time2);

	TimerThread::Handle TimerThread::scheduleTask(		TimerThread::Handle TimerThread::scheduleTask(
	std::string_view tag, TimerCallback&& func, Duration timeoutDuration) {		std::string_view tag, TimerCallback&& func,
			Duration timeoutDuration, Duration secondChanceDuration) {
	const auto now = std::chrono::system_clock::now();		const auto now = std::chrono::system_clock::now();
	auto request = std::make_shared<const Request>(now, now +		auto request = std::make_shared<const Request>(now, now +
	std::chrono::duration_cast<std::chrono::system_clock::duration>(timeoutDuration),		std::chrono::duration_cast<std::chrono::system_clock::duration>(timeoutDuration),
	gettid(), tag);		secondChanceDuration, gettid(), tag);
	return mMonitorThread.add(std::move(request), std::move(func), timeoutDuration);		return mMonitorThread.add(std::move(request), std::move(func), timeoutDuration);
	}		}

	TimerThread::Handle TimerThread::trackTask(std::string_view tag) {		TimerThread::Handle TimerThread::trackTask(std::string_view tag) {
	const auto now = std::chrono::system_clock::now();		const auto now = std::chrono::system_clock::now();
	auto request = std::make_shared<const Request>(now, now, gettid(), tag);		auto request = std::make_shared<const Request>(now, now,
			Duration{} /* secondChanceDuration */, gettid(), tag);
	return mNoTimeoutMap.add(std::move(request));		return mNoTimeoutMap.add(std::move(request));
	}		}

	@@ -86,6 +89,8 @@ std::string TimerThread::toString(size_t retiredCount) const {

	return std::string("now ")		return std::string("now ")
	.append(formatTime(std::chrono::system_clock::now()))		.append(formatTime(std::chrono::system_clock::now()))
			.append("\nsecondChanceCount ")
			.append(std::to_string(mMonitorThread.getSecondChanceCount()))
	.append(analysisSummary)		.append(analysisSummary)
	.append("\ntimeout [ ")		.append("\ntimeout [ ")
	.append(requestsToString(timeoutRequests))		.append(requestsToString(timeoutRequests))
	@@ -288,16 +293,60 @@ TimerThread::MonitorThread::~MonitorThread() {
	void TimerThread::MonitorThread::threadFunc() {		void TimerThread::MonitorThread::threadFunc() {
	std::unique_lock _l(mMutex);		std::unique_lock _l(mMutex);
	while (!mShouldExit) {		while (!mShouldExit) {
			Handle nextDeadline = INVALID_HANDLE;
			Handle now = INVALID_HANDLE;
	if (!mMonitorRequests.empty()) {		if (!mMonitorRequests.empty()) {
	Handle nextDeadline = mMonitorRequests.begin()->first;		nextDeadline = mMonitorRequests.begin()->first;
	if (nextDeadline < std::chrono::steady_clock::now()) {		now = std::chrono::steady_clock::now();
			if (nextDeadline < now) {
			auto node = mMonitorRequests.extract(mMonitorRequests.begin());
	// Deadline has expired, handle the request.		// Deadline has expired, handle the request.
			auto secondChanceDuration = node.mapped().first->secondChanceDuration;
			if (secondChanceDuration.count() != 0) {
			// We now apply the second chance duration to find the clock
			// monotonic second deadline. The unique key is then the
			// pair<second_deadline, first_deadline>.
			//
			// The second chance prevents a false timeout should there be
			// any clock monotonic advancement during suspend.
			auto newHandle = now + secondChanceDuration;
			ALOGD("%s: TimeCheck second chance applied for %s",
			__func__, node.mapped().first->tag.c_str()); // should be rare event.
			mSecondChanceRequests.emplace_hint(mSecondChanceRequests.end(),
			std::make_pair(newHandle, nextDeadline),
			std::move(node.mapped()));
			// increment second chance counter.
			mSecondChanceCount.fetch_add(1 /* arg */, std::memory_order_relaxed);
			} else {
	{		{
	auto node = mMonitorRequests.extract(mMonitorRequests.begin());
	_l.unlock();		_l.unlock();
	// We add Request to retired queue early so that it can be dumped out.		// We add Request to retired queue early so that it can be dumped out.
	mTimeoutQueue.add(std::move(node.mapped().first));		mTimeoutQueue.add(std::move(node.mapped().first));
	node.mapped().second(nextDeadline);		node.mapped().second(nextDeadline);
			// Caution: we don't hold lock when we call TimerCallback,
			// but this is the timeout case! We will crash soon,
			// maybe before returning.
			// anything left over is released here outside lock.
			}
			// reacquire the lock - if something was added, we loop immediately to check.
			_l.lock();
			}
			// always process expiring monitor requests first.
			continue;
			}
			}
			// now process any second chance requests.
			if (!mSecondChanceRequests.empty()) {
			Handle secondDeadline = mSecondChanceRequests.begin()->first.first;
			if (now == INVALID_HANDLE) now = std::chrono::steady_clock::now();
			if (secondDeadline < now) {
			auto node = mSecondChanceRequests.extract(mSecondChanceRequests.begin());
			{
			_l.unlock();
			// We add Request to retired queue early so that it can be dumped out.
			mTimeoutQueue.add(std::move(node.mapped().first));
			const Handle originalHandle = node.key().second;
			node.mapped().second(originalHandle);
	// Caution: we don't hold lock when we call TimerCallback.		// Caution: we don't hold lock when we call TimerCallback.
	// This is benign issue - we permit concurrent operations		// This is benign issue - we permit concurrent operations
	// while in the callback to the MonitorQueue.		// while in the callback to the MonitorQueue.
	@@ -308,6 +357,14 @@ void TimerThread::MonitorThread::threadFunc() {
	_l.lock();		_l.lock();
	continue;		continue;
	}		}
			// update the deadline.
			if (nextDeadline == INVALID_HANDLE) {
			nextDeadline = secondDeadline;
			} else {
			nextDeadline = std::min(nextDeadline, secondDeadline);
			}
			}
			if (nextDeadline != INVALID_HANDLE) {
	mCond.wait_until(_l, nextDeadline);		mCond.wait_until(_l, nextDeadline);
	} else {		} else {
	mCond.wait(_l);		mCond.wait(_l);
	@@ -319,22 +376,36 @@ TimerThread::Handle TimerThread::MonitorThread::add(
	std::shared_ptr<const Request> request, TimerCallback&& func, Duration timeout) {		std::shared_ptr<const Request> request, TimerCallback&& func, Duration timeout) {
	std::lock_guard _l(mMutex);		std::lock_guard _l(mMutex);
	const Handle handle = getUniqueHandle_l(timeout);		const Handle handle = getUniqueHandle_l(timeout);
	mMonitorRequests.emplace(handle, std::make_pair(std::move(request), std::move(func)));		mMonitorRequests.emplace_hint(mMonitorRequests.end(),
			handle, std::make_pair(std::move(request), std::move(func)));
	mCond.notify_all();		mCond.notify_all();
	return handle;		return handle;
	}		}

	std::shared_ptr<const TimerThread::Request> TimerThread::MonitorThread::remove(Handle handle) {		std::shared_ptr<const TimerThread::Request> TimerThread::MonitorThread::remove(Handle handle) {
			std::pair<std::shared_ptr<const Request>, TimerCallback> data;
	std::unique_lock ul(mMutex);		std::unique_lock ul(mMutex);
	const auto it = mMonitorRequests.find(handle);		if (const auto it = mMonitorRequests.find(handle);
	if (it == mMonitorRequests.end()) {		it != mMonitorRequests.end()) {
	return {};		data = std::move(it->second);
	}
	std::shared_ptr<const TimerThread::Request> request = std::move(it->second.first);
	TimerCallback func = std::move(it->second.second);
	mMonitorRequests.erase(it);		mMonitorRequests.erase(it);
	ul.unlock(); // manually release lock here so func is released outside of lock.		ul.unlock(); // manually release lock here so func (data.second)
	return request;		// is released outside of lock.
			return data.first; // request
			}

			// this check is O(N), but since the second chance requests are ordered
			// in terms of earliest expiration time, we would expect better than average results.
			for (auto it = mSecondChanceRequests.begin(); it != mSecondChanceRequests.end(); ++it) {
			if (it->first.second == handle) {
			data = std::move(it->second);
			mSecondChanceRequests.erase(it);
			ul.unlock(); // manually release lock here so func (data.second)
			// is released outside of lock.
			return data.first; // request
			}
			}
			return {};
	}		}

	void TimerThread::MonitorThread::copyRequests(		void TimerThread::MonitorThread::copyRequests(
	@@ -343,6 +414,13 @@ void TimerThread::MonitorThread::copyRequests(
	for (const auto &[deadline, monitorpair] : mMonitorRequests) {		for (const auto &[deadline, monitorpair] : mMonitorRequests) {
	requests.emplace_back(monitorpair.first);		requests.emplace_back(monitorpair.first);
	}		}
			// we combine the second map with the first map - this is
			// everything that is pending on the monitor thread.
			// The second map will be older than the first map so this
			// is in order.
			for (const auto &[deadline, monitorpair] : mSecondChanceRequests) {
			requests.emplace_back(monitorpair.first);
			}
	}		}

	} // namespace android::mediautils		} // namespace android::mediautils

media/utils/fuzzers/TimeCheckFuzz.cpp

+3 −1

Original line number	Original line	Diff line number	Diff line
	@@ -48,7 +48,9 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
	std::string name = data_provider.ConsumeRandomLengthString(kMaxStringLen);		std::string name = data_provider.ConsumeRandomLengthString(kMaxStringLen);

	// 3. The constructor, which is fuzzed here:		// 3. The constructor, which is fuzzed here:
	android::mediautils::TimeCheck timeCheck(name.c_str(), {} /* onTimer */, timeoutMs);		android::mediautils::TimeCheck timeCheck(name.c_str(), {} /* onTimer */,
			std::chrono::milliseconds(timeoutMs),
			{} /* secondChanceDuration /, true / crashOnTimeout */);
	// We will leave some buffer to avoid sleeping too long		// We will leave some buffer to avoid sleeping too long
	uint8_t sleep_amount_ms = data_provider.ConsumeIntegralInRange<uint8_t>(0, timeoutMs / 2);		uint8_t sleep_amount_ms = data_provider.ConsumeIntegralInRange<uint8_t>(0, timeoutMs / 2);

media/utils/include/mediautils/TimeCheck.h

+21 −5

Original line number	Original line	Diff line number	Diff line
	@@ -16,6 +16,7 @@

	#pragma once		#pragma once

			#include <chrono>
	#include <vector>		#include <vector>

	#include <mediautils/TimerThread.h>		#include <mediautils/TimerThread.h>
	@@ -44,7 +45,16 @@ class TimeCheck {
	// The default timeout is chosen to be less than system server watchdog timeout		// The default timeout is chosen to be less than system server watchdog timeout
	// Note: kDefaultTimeOutMs should be no less than 2 seconds, otherwise spurious timeouts		// Note: kDefaultTimeOutMs should be no less than 2 seconds, otherwise spurious timeouts
	// may occur with system suspend.		// may occur with system suspend.
	static constexpr uint32_t kDefaultTimeOutMs = 5000;		static constexpr TimeCheck::Duration kDefaultTimeoutDuration = std::chrono::milliseconds(3000);

			// Due to suspend abort not incrementing the monotonic clock,
			// we allow another second chance timeout after the first timeout expires.
			//
			// The total timeout is therefore kDefaultTimeoutDuration + kDefaultSecondChanceDuration,
			// and the result is more stable when the monotonic clock increments during suspend.
			//
			static constexpr TimeCheck::Duration kDefaultSecondChanceDuration =
			std::chrono::milliseconds(2000);

	/**		/**
	* TimeCheck is a RAII object which will notify a callback		* TimeCheck is a RAII object which will notify a callback
	@@ -64,14 +74,18 @@ class TimeCheck {
	* to block for callback completion if it is already in progress		* to block for callback completion if it is already in progress
	* (for maximum concurrency and reduced deadlock potential), so use proper		* (for maximum concurrency and reduced deadlock potential), so use proper
	* lifetime analysis (e.g. shared or weak pointers).		* lifetime analysis (e.g. shared or weak pointers).
	* \param requestedTimeoutMs timeout in milliseconds.		* \param requestedTimeoutDuration timeout in milliseconds.
	* A zero timeout means no timeout is set -		* A zero timeout means no timeout is set -
	* the callback is called only when		* the callback is called only when
	* the TimeCheck object is destroyed or leaves scope.		* the TimeCheck object is destroyed or leaves scope.
			* \param secondChanceDuration additional milliseconds to wait if the first timeout expires.
			* This is used to prevent false timeouts if the steady (monotonic)
			* clock advances on aborted suspend.
	* \param crashOnTimeout true if the object issues an abort on timeout.		* \param crashOnTimeout true if the object issues an abort on timeout.
	*/		*/
	explicit TimeCheck(std::string_view tag, OnTimerFunc&& onTimer = {},		explicit TimeCheck(std::string_view tag, OnTimerFunc&& onTimer,
	uint32_t requestedTimeoutMs = kDefaultTimeOutMs, bool crashOnTimeout = true);		Duration requestedTimeoutDuration, Duration secondChanceDuration,
			bool crashOnTimeout);

	TimeCheck() = default;		TimeCheck() = default;
	// Remove copy constructors as there should only be one call to the destructor.		// Remove copy constructors as there should only be one call to the destructor.
	@@ -91,13 +105,14 @@ class TimeCheck {
	public:		public:
	template <typename S, typename F>		template <typename S, typename F>
	TimeCheckHandler(S&& _tag, F&& _onTimer, bool _crashOnTimeout,		TimeCheckHandler(S&& _tag, F&& _onTimer, bool _crashOnTimeout,
	Duration _timeoutDuration,		Duration _timeoutDuration, Duration _secondChanceDuration,
	std::chrono::system_clock::time_point _startSystemTime,		std::chrono::system_clock::time_point _startSystemTime,
	pid_t _tid)		pid_t _tid)
	: tag(std::forward<S>(_tag))		: tag(std::forward<S>(_tag))
	, onTimer(std::forward<F>(_onTimer))		, onTimer(std::forward<F>(_onTimer))
	, crashOnTimeout(_crashOnTimeout)		, crashOnTimeout(_crashOnTimeout)
	, timeoutDuration(_timeoutDuration)		, timeoutDuration(_timeoutDuration)
			, secondChanceDuration(_secondChanceDuration)
	, startSystemTime(_startSystemTime)		, startSystemTime(_startSystemTime)
	, tid(_tid)		, tid(_tid)
	{}		{}
	@@ -105,6 +120,7 @@ class TimeCheck {
	const OnTimerFunc onTimer;		const OnTimerFunc onTimer;
	const bool crashOnTimeout;		const bool crashOnTimeout;
	const Duration timeoutDuration;		const Duration timeoutDuration;
			const Duration secondChanceDuration;
	const std::chrono::system_clock::time_point startSystemTime;		const std::chrono::system_clock::time_point startSystemTime;
	const pid_t tid;		const pid_t tid;