Remove OBBs from state list when volume unmounted
Don't keep tracking OBBs when the volume they're located on goes away.
Remove them from our state tracking maps and then send a notification to
any listener that is still around.
Add a dump handler to MountService so the state of the mount lists
can be inspected.
Change the API to just make a callback directly to the change listener
when mount is called when it's already mounted or unmount called when
it's already unmounted.
Change-Id: Idb4afbb943ca5ca775825f908bff334e3ce1cfcc
diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java
index 265d6138..ea280a5 100644
--- a/services/java/com/android/server/MountService.java
+++ b/services/java/com/android/server/MountService.java
@@ -46,13 +46,18 @@
import android.os.storage.StorageResultCode;
import android.util.Slog;
+import java.io.FileDescriptor;
import java.io.IOException;
+import java.io.PrintWriter;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
/**
* MountService implements back-end services for platform storage
@@ -181,6 +186,22 @@
token.asBinder().unlinkToDeath(this, 0);
}
+
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder("ObbState{");
+ sb.append("filename=");
+ sb.append(filename);
+ sb.append(",token=");
+ sb.append(token.toString());
+ sb.append(",callerUid=");
+ sb.append(callerUid);
+ sb.append(",mounted=");
+ sb.append(mounted);
+ sb.append('}');
+ return sb.toString();
+ }
+
}
// OBB Action Handler
@@ -475,6 +496,34 @@
} else if (Environment.MEDIA_MOUNTED.equals(state)) {
mPms.updateExternalMediaStatus(true, false);
}
+
+ // Remove all OBB mappings and listeners from this path
+ synchronized (mObbMounts) {
+ final List<ObbState> obbStatesToRemove = new LinkedList<ObbState>();
+
+ final Iterator<Entry<String, ObbState>> i = mObbPathToStateMap.entrySet().iterator();
+ while (i.hasNext()) {
+ final Entry<String, ObbState> obbEntry = i.next();
+
+ // If this entry's source file is in the volume path that got
+ // unmounted, remove it because it's no longer valid.
+ if (obbEntry.getKey().startsWith(path)) {
+ obbStatesToRemove.add(obbEntry.getValue());
+ }
+ }
+
+ for (final ObbState obbState : obbStatesToRemove) {
+ removeObbState(obbState);
+
+ try {
+ obbState.token.onObbResult(obbState.filename, Environment.MEDIA_UNMOUNTED);
+ } catch (RemoteException e) {
+ Slog.i(TAG, "Couldn't send unmount notification for OBB: "
+ + obbState.filename);
+ }
+ }
+ }
+
String oldState = mLegacyState;
mLegacyState = state;
@@ -1494,8 +1543,14 @@
public boolean isObbMounted(String filename) {
synchronized (mObbMounts) {
- return mObbPathToStateMap.containsKey(filename);
+ final ObbState obbState = mObbPathToStateMap.get(filename);
+ if (obbState != null) {
+ synchronized (obbState) {
+ return obbState.mounted;
+ }
+ }
}
+ return false;
}
public void mountObb(String filename, String key, IObbActionListener token) {
@@ -1512,7 +1567,12 @@
synchronized (mObbMounts) {
if (isObbMounted(filename)) {
- throw new IllegalArgumentException("OBB file is already mounted");
+ try {
+ token.onObbResult(filename, Environment.MEDIA_MOUNTED);
+ } catch (RemoteException e) {
+ Slog.d(TAG, "Could not send unmount notification for: " + filename);
+ }
+ return;
}
final int callerUid = Binder.getCallingUid();
@@ -1526,7 +1586,7 @@
Slog.e(TAG, "Failed to link to listener death");
}
- MountObbAction action = new MountObbAction(obbState, key);
+ ObbAction action = new MountObbAction(obbState, key);
mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
if (DEBUG_OBB)
@@ -1544,8 +1604,14 @@
synchronized (mObbMounts) {
if (!isObbMounted(filename)) {
- throw new IllegalArgumentException("OBB is not mounted");
+ try {
+ token.onObbResult(filename, Environment.MEDIA_UNMOUNTED);
+ } catch (RemoteException e) {
+ Slog.d(TAG, "Could not send unmount notification for: " + filename);
+ }
+ return;
}
+
obbState = mObbPathToStateMap.get(filename);
if (Binder.getCallingUid() != obbState.callerUid) {
@@ -1555,7 +1621,7 @@
}
}
- UnmountObbAction action = new UnmountObbAction(obbState, force);
+ ObbAction action = new UnmountObbAction(obbState, force);
mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
if (DEBUG_OBB)
@@ -1587,6 +1653,13 @@
}
}
+ private void replaceObbState(ObbState oldObbState, ObbState newObbState) {
+ synchronized (mObbMounts) {
+ removeObbState(oldObbState);
+ addObbState(newObbState);
+ }
+ }
+
private class ObbActionHandler extends Handler {
private boolean mBound = false;
private List<ObbAction> mActions = new LinkedList<ObbAction>();
@@ -1760,6 +1833,29 @@
abstract void handleExecute() throws RemoteException, IOException;
abstract void handleError();
+
+ protected ObbInfo getObbInfo() throws IOException {
+ ObbInfo obbInfo;
+ try {
+ obbInfo = mContainerService.getObbInfo(mObbState.filename);
+ } catch (RemoteException e) {
+ Slog.d(TAG, "Couldn't call DefaultContainerService to fetch OBB info for "
+ + mObbState.filename);
+ obbInfo = null;
+ }
+ if (obbInfo == null) {
+ throw new IOException("Couldn't read OBB file: " + mObbState.filename);
+ }
+ return obbInfo;
+ }
+
+ protected void sendNewStatusOrIgnore(String filename, String status) {
+ try {
+ mObbState.token.onObbResult(filename, status);
+ } catch (RemoteException e) {
+ Slog.w(TAG, "MountServiceListener went away while calling onObbStateChanged");
+ }
+ }
}
class MountObbAction extends ObbAction {
@@ -1770,10 +1866,42 @@
mKey = key;
}
- public void handleExecute() throws RemoteException, IOException {
- final ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
- if (obbInfo == null) {
- throw new IOException("Couldn't read OBB file: " + mObbState.filename);
+ public void handleExecute() throws IOException {
+ final ObbInfo obbInfo = getObbInfo();
+
+ /*
+ * If someone tried to trick us with some weird characters, rectify
+ * it here.
+ */
+ if (!mObbState.filename.equals(obbInfo.filename)) {
+ if (DEBUG_OBB)
+ Slog.i(TAG, "OBB filename " + mObbState.filename + " is actually "
+ + obbInfo.filename);
+
+ synchronized (mObbMounts) {
+ /*
+ * If the real filename is already mounted, discard this
+ * state and notify the caller that the OBB is already
+ * mounted.
+ */
+ if (isObbMounted(obbInfo.filename)) {
+ if (DEBUG_OBB)
+ Slog.i(TAG, "OBB already mounted as " + obbInfo.filename);
+
+ removeObbState(mObbState);
+ sendNewStatusOrIgnore(obbInfo.filename, Environment.MEDIA_MOUNTED);
+ return;
+ }
+
+ /*
+ * It's not already mounted, so we have to replace the state
+ * with the state containing the actual filename.
+ */
+ ObbState newObbState = new ObbState(obbInfo.filename, mObbState.token,
+ mObbState.callerUid);
+ replaceObbState(mObbState, newObbState);
+ mObbState = newObbState;
+ }
}
if (!isUidOwnerOfPackageOrSystem(obbInfo.packageName, mObbState.callerUid)) {
@@ -1784,42 +1912,47 @@
mKey = "none";
}
- int rc = StorageResultCode.OperationSucceeded;
- String cmd = String.format("obb mount %s %s %d", mObbState.filename, mKey,
- mObbState.callerUid);
- try {
- mConnector.doCommand(cmd);
- } catch (NativeDaemonConnectorException e) {
- int code = e.getCode();
- if (code != VoldResponseCode.OpFailedStorageBusy) {
- rc = StorageResultCode.OperationFailedInternalError;
+ boolean mounted = false;
+ int rc;
+ synchronized (mObbState) {
+ if (mObbState.mounted) {
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_MOUNTED);
+ return;
+ }
+
+ rc = StorageResultCode.OperationSucceeded;
+ String cmd = String.format("obb mount %s %s %d", mObbState.filename, mKey,
+ mObbState.callerUid);
+ try {
+ mConnector.doCommand(cmd);
+ } catch (NativeDaemonConnectorException e) {
+ int code = e.getCode();
+ if (code != VoldResponseCode.OpFailedStorageBusy) {
+ rc = StorageResultCode.OperationFailedInternalError;
+ }
+ }
+
+ if (rc == StorageResultCode.OperationSucceeded) {
+ mObbState.mounted = mounted = true;
}
}
- if (rc == StorageResultCode.OperationSucceeded) {
- try {
- mObbState.token.onObbResult(mObbState.filename, Environment.MEDIA_MOUNTED);
- } catch (RemoteException e) {
- Slog.w(TAG, "MountServiceListener went away while calling onObbStateChanged");
- }
+ if (mounted) {
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_MOUNTED);
} else {
Slog.e(TAG, "Couldn't mount OBB file: " + rc);
// We didn't succeed, so remove this from the mount-set.
removeObbState(mObbState);
- mObbState.token.onObbResult(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_UNMOUNTED);
}
}
public void handleError() {
removeObbState(mObbState);
- try {
- mObbState.token.onObbResult(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
- } catch (RemoteException e) {
- Slog.e(TAG, "Couldn't send back OBB mount error for " + mObbState.filename);
- }
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
}
@Override
@@ -1845,51 +1978,66 @@
mForceUnmount = force;
}
- public void handleExecute() throws RemoteException, IOException {
- final ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
- if (obbInfo == null) {
- throw new IOException("Couldn't read OBB file: " + mObbState.filename);
- }
+ public void handleExecute() throws IOException {
+ final ObbInfo obbInfo = getObbInfo();
- int rc = StorageResultCode.OperationSucceeded;
- String cmd = String.format("obb unmount %s%s", mObbState.filename,
- (mForceUnmount ? " force" : ""));
- try {
- mConnector.doCommand(cmd);
- } catch (NativeDaemonConnectorException e) {
- int code = e.getCode();
- if (code == VoldResponseCode.OpFailedStorageBusy) {
- rc = StorageResultCode.OperationFailedStorageBusy;
- } else {
- rc = StorageResultCode.OperationFailedInternalError;
+ /*
+ * If someone tried to trick us with some weird characters, rectify
+ * it here.
+ */
+ synchronized (mObbMounts) {
+ if (!isObbMounted(obbInfo.filename)) {
+ removeObbState(mObbState);
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_UNMOUNTED);
+ return;
+ }
+
+ if (!mObbState.filename.equals(obbInfo.filename)) {
+ removeObbState(mObbState);
+ mObbState = mObbPathToStateMap.get(obbInfo.filename);
}
}
- if (rc == StorageResultCode.OperationSucceeded) {
+ boolean unmounted = false;
+ synchronized (mObbState) {
+ if (!mObbState.mounted) {
+ sendNewStatusOrIgnore(obbInfo.filename, Environment.MEDIA_UNMOUNTED);
+ return;
+ }
+
+ int rc = StorageResultCode.OperationSucceeded;
+ String cmd = String.format("obb unmount %s%s", mObbState.filename,
+ (mForceUnmount ? " force" : ""));
+ try {
+ mConnector.doCommand(cmd);
+ } catch (NativeDaemonConnectorException e) {
+ int code = e.getCode();
+ if (code == VoldResponseCode.OpFailedStorageBusy) {
+ rc = StorageResultCode.OperationFailedStorageBusy;
+ } else {
+ rc = StorageResultCode.OperationFailedInternalError;
+ }
+ }
+
+ if (rc == StorageResultCode.OperationSucceeded) {
+ mObbState.mounted = false;
+ unmounted = true;
+ }
+ }
+
+ if (unmounted) {
removeObbState(mObbState);
- try {
- mObbState.token.onObbResult(mObbState.filename, Environment.MEDIA_UNMOUNTED);
- } catch (RemoteException e) {
- Slog.w(TAG, "MountServiceListener went away while calling onObbStateChanged");
- }
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_UNMOUNTED);
} else {
- try {
- mObbState.token.onObbResult(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
- } catch (RemoteException e) {
- Slog.w(TAG, "MountServiceListener went away while calling onObbStateChanged");
- }
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_MOUNTED);
}
}
public void handleError() {
removeObbState(mObbState);
- try {
- mObbState.token.onObbResult(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
- } catch (RemoteException e) {
- Slog.e(TAG, "Couldn't send back OBB unmount error for " + mObbState.filename);
- }
+ sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
}
@Override
@@ -1908,5 +2056,27 @@
return sb.toString();
}
}
+
+ @Override
+ protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
+ if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) != PackageManager.PERMISSION_GRANTED) {
+ pw.println("Permission Denial: can't dump ActivityManager from from pid="
+ + Binder.getCallingPid() + ", uid=" + Binder.getCallingUid()
+ + " without permission " + android.Manifest.permission.DUMP);
+ return;
+ }
+
+ pw.println(" mObbMounts:");
+
+ synchronized (mObbMounts) {
+ final Collection<List<ObbState>> obbStateLists = mObbMounts.values();
+
+ for (final List<ObbState> obbStates : obbStateLists) {
+ for (final ObbState obbState : obbStates) {
+ pw.print(" "); pw.println(obbState.toString());
+ }
+ }
+ }
+ }
}