Merge "Additional lock to avoid access to deleted object." into rvc-dev (28d12368) · Commits · e / os / android_frameworks_base

services/incremental/IncrementalService.cpp

+15 −13

Original line number	Original line	Diff line number	Diff line
	@@ -1670,9 +1670,15 @@ IncrementalService::DataLoaderStub::~DataLoaderStub() = default;

	void IncrementalService::DataLoaderStub::cleanupResources() {		void IncrementalService::DataLoaderStub::cleanupResources() {
	requestDestroy();		requestDestroy();

			auto now = Clock::now();

			std::unique_lock lock(mMutex);
	mParams = {};		mParams = {};
	mControl = {};		mControl = {};
	waitForStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED, std::chrono::seconds(60));		mStatusCondition.wait_until(lock, now + 60s, [this] {
			return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
			});
	mListener = {};		mListener = {};
	mId = kInvalidStorageId;		mId = kInvalidStorageId;
	}		}
	@@ -1706,7 +1712,7 @@ bool IncrementalService::DataLoaderStub::requestDestroy() {
	bool IncrementalService::DataLoaderStub::setTargetStatus(int newStatus) {		bool IncrementalService::DataLoaderStub::setTargetStatus(int newStatus) {
	int oldStatus, curStatus;		int oldStatus, curStatus;
	{		{
	std::unique_lock lock(mStatusMutex);		std::unique_lock lock(mMutex);
	oldStatus = mTargetStatus;		oldStatus = mTargetStatus;
	curStatus = mCurrentStatus;		curStatus = mCurrentStatus;
	setTargetStatusLocked(newStatus);		setTargetStatusLocked(newStatus);
	@@ -1721,13 +1727,6 @@ void IncrementalService::DataLoaderStub::setTargetStatusLocked(int status) {
	mTargetStatusTs = Clock::now();		mTargetStatusTs = Clock::now();
	}		}

	bool IncrementalService::DataLoaderStub::waitForStatus(int status, Clock::duration duration) {
	auto now = Clock::now();
	std::unique_lock lock(mStatusMutex);
	return mStatusCondition.wait_until(lock, now + duration,
	[this, status] { return mCurrentStatus == status; });
	}

	bool IncrementalService::DataLoaderStub::bind() {		bool IncrementalService::DataLoaderStub::bind() {
	bool result = false;		bool result = false;
	auto status = mService.mDataLoaderManager->bindToDataLoader(mId, mParams, this, &result);		auto status = mService.mDataLoaderManager->bindToDataLoader(mId, mParams, this, &result);
	@@ -1776,7 +1775,7 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
	int currentStatus;		int currentStatus;
	int targetStatus;		int targetStatus;
	{		{
	std::unique_lock lock(mStatusMutex);		std::unique_lock lock(mMutex);
	currentStatus = mCurrentStatus;		currentStatus = mCurrentStatus;
	targetStatus = mTargetStatus;		targetStatus = mTargetStatus;
	}		}
	@@ -1828,8 +1827,9 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
	}		}

	int targetStatus, oldStatus;		int targetStatus, oldStatus;
			DataLoaderStatusListener listener;
	{		{
	std::unique_lock lock(mStatusMutex);		std::unique_lock lock(mMutex);
	if (mCurrentStatus == newStatus) {		if (mCurrentStatus == newStatus) {
	return binder::Status::ok();		return binder::Status::ok();
	}		}
	@@ -1838,6 +1838,8 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
	mCurrentStatus = newStatus;		mCurrentStatus = newStatus;
	targetStatus = mTargetStatus;		targetStatus = mTargetStatus;

			listener = mListener;

	if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE) {		if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE) {
	// For unavailable, reset target status.		// For unavailable, reset target status.
	setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE);		setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE);
	@@ -1847,8 +1849,8 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
	LOG(DEBUG) << "Current status update for DataLoader " << mId << ": " << oldStatus << " -> "		LOG(DEBUG) << "Current status update for DataLoader " << mId << ": " << oldStatus << " -> "
	<< newStatus << " (target " << targetStatus << ")";		<< newStatus << " (target " << targetStatus << ")";

	if (mListener) {		if (listener) {
	mListener->onStatusChanged(mountId, newStatus);		listener->onStatusChanged(mountId, newStatus);
	}		}

	fsmStep();		fsmStep();

services/incremental/IncrementalService.h

+2 −2

Original line number	Original line	Diff line number	Diff line
	@@ -188,17 +188,17 @@ private:

	bool setTargetStatus(int status);		bool setTargetStatus(int status);
	void setTargetStatusLocked(int status);		void setTargetStatusLocked(int status);
	bool waitForStatus(int status, Clock::duration duration);

	bool fsmStep();		bool fsmStep();

	IncrementalService& mService;		IncrementalService& mService;

			std::mutex mMutex;
	MountId mId = kInvalidStorageId;		MountId mId = kInvalidStorageId;
	content::pm::DataLoaderParamsParcel mParams;		content::pm::DataLoaderParamsParcel mParams;
	content::pm::FileSystemControlParcel mControl;		content::pm::FileSystemControlParcel mControl;
	DataLoaderStatusListener mListener;		DataLoaderStatusListener mListener;

	std::mutex mStatusMutex;
	std::condition_variable mStatusCondition;		std::condition_variable mStatusCondition;
	int mCurrentStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;		int mCurrentStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
	int mTargetStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;		int mTargetStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;