Merge "Additional lock to avoid access to deleted object." into rvc-dev

This commit is contained in:
Alex Buynytskyy
2020-05-05 05:12:02 +00:00
committed by Android (Google) Code Review
2 changed files with 17 additions and 15 deletions

View File

@@ -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();

View File

@@ -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;