Bug 1166320 - Make volume service safer to use off main thread. r=dhylands

This commit is contained in:
Andrew Osmond 2015-08-20 12:47:23 -04:00
parent a8a5cb3ed6
commit 2758829a0c
4 changed files with 61 additions and 90 deletions

View File

@ -66,6 +66,22 @@ nsVolume::nsVolume(const Volume* aVolume)
{
}
nsVolume::nsVolume(const nsVolume* aVolume)
: mName(aVolume->mName),
mMountPoint(aVolume->mMountPoint),
mState(aVolume->mState),
mMountGeneration(aVolume->mMountGeneration),
mMountLocked(aVolume->mMountLocked),
mIsFake(aVolume->mIsFake),
mIsMediaPresent(aVolume->mIsMediaPresent),
mIsSharing(aVolume->mIsSharing),
mIsFormatting(aVolume->mIsFormatting),
mIsUnmounting(aVolume->mIsUnmounting),
mIsRemovable(aVolume->mIsRemovable),
mIsHotSwappable(aVolume->mIsHotSwappable)
{
}
void nsVolume::Dump(const char* aLabel) const
{
LOG("%s: Volume: %s is %s and %s @ %s gen %d locked %d",
@ -333,40 +349,28 @@ nsVolume::LogState() const
LOG("nsVolume: %s state %s", NameStr().get(), StateStr());
}
void nsVolume::Set(nsIVolume* aVolume)
void nsVolume::UpdateMountLock(nsVolume* aOldVolume)
{
MOZ_ASSERT(NS_IsMainThread());
aVolume->GetName(mName);
aVolume->GetMountPoint(mMountPoint);
aVolume->GetState(&mState);
aVolume->GetIsFake(&mIsFake);
aVolume->GetIsMediaPresent(&mIsMediaPresent);
aVolume->GetIsSharing(&mIsSharing);
aVolume->GetIsFormatting(&mIsFormatting);
aVolume->GetIsUnmounting(&mIsUnmounting);
aVolume->GetIsRemovable(&mIsRemovable);
aVolume->GetIsHotSwappable(&mIsHotSwappable);
int32_t volMountGeneration;
aVolume->GetMountGeneration(&volMountGeneration);
bool oldMountLocked = aOldVolume ? aOldVolume->mMountLocked : false;
if (mState != nsIVolume::STATE_MOUNTED) {
// Since we're not in the mounted state, we need to
// forgot whatever mount generation we may have had.
mMountGeneration = -1;
return;
}
if (mMountGeneration == volMountGeneration) {
// No change in mount generation, nothing else to do
mMountLocked = oldMountLocked;
return;
}
mMountGeneration = volMountGeneration;
int32_t oldMountGeneration = aOldVolume ? aOldVolume->mMountGeneration : -1;
if (mMountGeneration == oldMountGeneration) {
// No change in mount generation, nothing else to do
mMountLocked = oldMountLocked;
return;
}
if (!XRE_IsParentProcess()) {
// Child processes just track the state, not maintain it.
aVolume->GetIsMountLocked(&mMountLocked);
return;
}

View File

@ -24,6 +24,9 @@ public:
// This constructor is used by the UpdateVolumeRunnable constructor
nsVolume(const Volume* aVolume);
// This constructor is used by nsVolumeService::SetFakeVolumeState
nsVolume(const nsVolume* aVolume);
// This constructor is used by ContentChild::RecvFileSystemUpdate which is
// used to update the volume cache maintained in the child process.
nsVolume(const nsAString& aName, const nsAString& aMountPoint,
@ -47,25 +50,8 @@ public:
{
}
// This constructor is used by nsVolumeService::FindAddVolumeByName, and
// will be followed shortly by a Set call.
nsVolume(const nsAString& aName)
: mName(aName),
mState(STATE_INIT),
mMountGeneration(-1),
mMountLocked(true), // Needs to agree with Volume::Volume
mIsFake(false),
mIsMediaPresent(false),
mIsSharing(false),
mIsFormatting(false),
mIsUnmounting(false),
mIsRemovable(false),
mIsHotSwappable(false)
{
}
bool Equals(nsIVolume* aVolume);
void Set(nsIVolume* aVolume);
void UpdateMountLock(nsVolume* aOldVolume);
void LogState() const;

View File

@ -368,7 +368,7 @@ nsVolumeService::FindVolumeByMountLockName(const nsAString& aMountLockName)
}
already_AddRefed<nsVolume>
nsVolumeService::FindVolumeByName(const nsAString& aName)
nsVolumeService::FindVolumeByName(const nsAString& aName, nsVolume::Array::index_type* aIndex)
{
mArrayMonitor.AssertCurrentThreadOwns();
@ -377,52 +377,35 @@ nsVolumeService::FindVolumeByName(const nsAString& aName)
for (volIndex = 0; volIndex < numVolumes; volIndex++) {
nsRefPtr<nsVolume> vol = mVolumeArray[volIndex];
if (vol->Name().Equals(aName)) {
if (aIndex) {
*aIndex = volIndex;
}
return vol.forget();
}
}
return nullptr;
}
//static
already_AddRefed<nsVolume>
nsVolumeService::CreateOrFindVolumeByName(const nsAString& aName, bool aIsFake /*= false*/)
{
MonitorAutoLock autoLock(mArrayMonitor);
nsRefPtr<nsVolume> vol;
vol = FindVolumeByName(aName);
if (vol) {
return vol.forget();
}
// Volume not found - add a new one
vol = new nsVolume(aName);
vol->SetIsFake(aIsFake);
mVolumeArray.AppendElement(vol);
return vol.forget();
}
void
nsVolumeService::UpdateVolume(nsIVolume* aVolume, bool aNotifyObservers)
nsVolumeService::UpdateVolume(nsVolume* aVolume, bool aNotifyObservers)
{
MOZ_ASSERT(NS_IsMainThread());
nsString volName;
aVolume->GetName(volName);
bool aIsFake;
aVolume->GetIsFake(&aIsFake);
nsRefPtr<nsVolume> vol = CreateOrFindVolumeByName(volName, aIsFake);
if (vol->Equals(aVolume)) {
// Nothing has really changed. Don't bother telling anybody.
return;
{
MonitorAutoLock autoLock(mArrayMonitor);
nsVolume::Array::index_type volIndex;
nsRefPtr<nsVolume> vol = FindVolumeByName(aVolume->Name(), &volIndex);
if (!vol) {
mVolumeArray.AppendElement(aVolume);
} else if (vol->Equals(aVolume) || (!vol->IsFake() && aVolume->IsFake())) {
// Ignore if nothing changed or if a fake tries to override a real volume.
return;
} else {
mVolumeArray.ReplaceElementAt(volIndex, aVolume);
}
aVolume->UpdateMountLock(vol);
}
if (!vol->IsFake() && aIsFake) {
// Prevent an incoming fake volume from overriding an existing real volume.
return;
}
vol->Set(aVolume);
if (!aNotifyObservers) {
return;
}
@ -431,8 +414,8 @@ nsVolumeService::UpdateVolume(nsIVolume* aVolume, bool aNotifyObservers)
if (!obs) {
return;
}
NS_ConvertUTF8toUTF16 stateStr(vol->StateStr());
obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED, stateStr.get());
NS_ConvertUTF8toUTF16 stateStr(aVolume->StateStr());
obs->NotifyObservers(aVolume, NS_VOLUME_STATE_CHANGED, stateStr.get());
}
NS_IMETHODIMP
@ -471,11 +454,8 @@ nsVolumeService::SetFakeVolumeState(const nsAString& name, int32_t state)
return NS_ERROR_NOT_AVAILABLE;
}
// UpdateVolume expects the volume passed in to NOT be the
// same pointer as what CreateOrFindVolumeByName would return,
// which is why we allocate a temporary volume here.
nsRefPtr<nsVolume> volume = new nsVolume(name);
volume->Set(vol);
// Clone the existing volume so we can replace it
nsRefPtr<nsVolume> volume = new nsVolume(vol);
volume->SetState(state);
volume->LogState();
UpdateVolume(volume.get());
@ -502,15 +482,15 @@ nsVolumeService::RemoveFakeVolume(const nsAString& name)
void
nsVolumeService::RemoveVolumeByName(const nsAString& aName)
{
nsRefPtr<nsVolume> vol;
{
MonitorAutoLock autoLock(mArrayMonitor);
vol = FindVolumeByName(aName);
nsVolume::Array::index_type volIndex;
nsRefPtr<nsVolume> vol = FindVolumeByName(aName, &volIndex);
if (!vol) {
return;
}
mVolumeArray.RemoveElementAt(volIndex);
}
if (!vol) {
return;
}
mVolumeArray.RemoveElement(vol);
if (XRE_IsParentProcess()) {
nsCOMPtr<nsIObserverService> obs = GetObserverService();

View File

@ -47,7 +47,7 @@ public:
void DumpNoLock(const char* aLabel);
// To use this function, you have to create a new volume and pass it in.
void UpdateVolume(nsIVolume* aVolume, bool aNotifyObservers = true);
void UpdateVolume(nsVolume* aVolume, bool aNotifyObservers = true);
void UpdateVolumeIOThread(const Volume* aVolume);
void RecvVolumesFromParent(const nsTArray<dom::VolumeInfo>& aVolumes);
@ -61,8 +61,9 @@ private:
void CheckMountLock(const nsAString& aMountLockName,
const nsAString& aMountLockState);
already_AddRefed<nsVolume> FindVolumeByMountLockName(const nsAString& aMountLockName);
already_AddRefed<nsVolume> FindVolumeByName(const nsAString& aName);
already_AddRefed<nsVolume> CreateOrFindVolumeByName(const nsAString& aName, bool aIsFake = false);
already_AddRefed<nsVolume> FindVolumeByName(const nsAString& aName,
nsVolume::Array::index_type* aIndex = nullptr);
Monitor mArrayMonitor;
nsVolume::Array mVolumeArray;