OBB: rearrange to be entirely asynchronous
Rearrange structure of MountService handling of OBBs to be entirely
asynchronous so we don't rely on locking as much. We still need the
locking to support dumpsys which has been improved to output all the
data structures for OBBs.
Added more tests to cover more of the error return codes.
Oh and fix a logic inversion bug.
Change-Id: I34f541192dbbb1903b24825889b8fa8f43e6e2a9
diff --git a/api/current.xml b/api/current.xml
index 2a2200d..5fd7a1b 100644
--- a/api/current.xml
+++ b/api/current.xml
@@ -131704,9 +131704,97 @@
>
<parameter name="path" type="java.lang.String">
</parameter>
-<parameter name="state" type="java.lang.String">
+<parameter name="state" type="int">
</parameter>
</method>
+<field name="ERROR_ALREADY_MOUNTED"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="24"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="ERROR_COULD_NOT_MOUNT"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="21"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="ERROR_COULD_NOT_UNMOUNT"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="22"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="ERROR_INTERNAL"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="20"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="ERROR_NOT_MOUNTED"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="23"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="ERROR_PERMISSION_DENIED"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="25"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="MOUNTED"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="1"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
+<field name="UNMOUNTED"
+ type="int"
+ transient="false"
+ volatile="false"
+ value="2"
+ static="true"
+ final="true"
+ deprecated="not deprecated"
+ visibility="public"
+>
+</field>
</class>
<class name="StorageManager"
extends="java.lang.Object"
@@ -131741,8 +131829,6 @@
>
<parameter name="filename" type="java.lang.String">
</parameter>
-<exception name="IllegalArgumentException" type="java.lang.IllegalArgumentException">
-</exception>
</method>
<method name="mountObb"
return="boolean"
diff --git a/core/java/android/os/storage/IMountService.java b/core/java/android/os/storage/IMountService.java
index 60ea95c..467a0ac 100644
--- a/core/java/android/os/storage/IMountService.java
+++ b/core/java/android/os/storage/IMountService.java
@@ -484,7 +484,7 @@
* IObbActionListener to inform it of the terminal state of the
* call.
*/
- public void mountObb(String filename, String key, IObbActionListener token)
+ public void mountObb(String filename, String key, IObbActionListener token, int nonce)
throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
@@ -493,6 +493,7 @@
_data.writeString(filename);
_data.writeString(key);
_data.writeStrongBinder((token != null ? token.asBinder() : null));
+ _data.writeInt(nonce);
mRemote.transact(Stub.TRANSACTION_mountObb, _data, _reply, 0);
_reply.readException();
} finally {
@@ -508,8 +509,8 @@
* IObbActionListener to inform it of the terminal state of the
* call.
*/
- public void unmountObb(String filename, boolean force, IObbActionListener token)
- throws RemoteException {
+ public void unmountObb(String filename, boolean force, IObbActionListener token,
+ int nonce) throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
try {
@@ -517,6 +518,7 @@
_data.writeString(filename);
_data.writeInt((force ? 1 : 0));
_data.writeStrongBinder((token != null ? token.asBinder() : null));
+ _data.writeInt(nonce);
mRemote.transact(Stub.TRANSACTION_unmountObb, _data, _reply, 0);
_reply.readException();
} finally {
@@ -855,7 +857,9 @@
key = data.readString();
IObbActionListener observer;
observer = IObbActionListener.Stub.asInterface(data.readStrongBinder());
- mountObb(filename, key, observer);
+ int nonce;
+ nonce = data.readInt();
+ mountObb(filename, key, observer, nonce);
reply.writeNoException();
return true;
}
@@ -867,7 +871,9 @@
force = 0 != data.readInt();
IObbActionListener observer;
observer = IObbActionListener.Stub.asInterface(data.readStrongBinder());
- unmountObb(filename, force, observer);
+ int nonce;
+ nonce = data.readInt();
+ unmountObb(filename, force, observer, nonce);
reply.writeNoException();
return true;
}
@@ -979,7 +985,7 @@
* MountService will call back to the supplied IObbActionListener to inform
* it of the terminal state of the call.
*/
- public void mountObb(String filename, String key, IObbActionListener token)
+ public void mountObb(String filename, String key, IObbActionListener token, int nonce)
throws RemoteException;
/*
@@ -1023,7 +1029,7 @@
* MountService will call back to the supplied IObbActionListener to inform
* it of the terminal state of the call.
*/
- public void unmountObb(String filename, boolean force, IObbActionListener token)
+ public void unmountObb(String filename, boolean force, IObbActionListener token, int nonce)
throws RemoteException;
/*
diff --git a/core/java/android/os/storage/IObbActionListener.java b/core/java/android/os/storage/IObbActionListener.java
index 2c098ac..d6fa58a 100644
--- a/core/java/android/os/storage/IObbActionListener.java
+++ b/core/java/android/os/storage/IObbActionListener.java
@@ -69,9 +69,11 @@
data.enforceInterface(DESCRIPTOR);
String filename;
filename = data.readString();
- String status;
- status = data.readString();
- this.onObbResult(filename, status);
+ int nonce;
+ nonce = data.readInt();
+ int status;
+ status = data.readInt();
+ this.onObbResult(filename, nonce, status);
reply.writeNoException();
return true;
}
@@ -101,13 +103,15 @@
* on
* @param returnCode status of the operation
*/
- public void onObbResult(String filename, String status) throws RemoteException {
+ public void onObbResult(String filename, int nonce, int status)
+ throws RemoteException {
Parcel _data = Parcel.obtain();
Parcel _reply = Parcel.obtain();
try {
_data.writeInterfaceToken(DESCRIPTOR);
_data.writeString(filename);
- _data.writeString(status);
+ _data.writeInt(nonce);
+ _data.writeInt(status);
mRemote.transact(Stub.TRANSACTION_onObbResult, _data, _reply, 0);
_reply.readException();
} finally {
@@ -124,7 +128,8 @@
* Return from an OBB action result.
*
* @param filename the path to the OBB the operation was performed on
- * @param returnCode status of the operation
+ * @param nonce identifier that is meaningful to the receiver
+ * @param status status code as defined in {@link OnObbStateChangeListener}
*/
- public void onObbResult(String filename, String status) throws RemoteException;
+ public void onObbResult(String filename, int nonce, int status) throws RemoteException;
}
diff --git a/core/java/android/os/storage/OnObbStateChangeListener.java b/core/java/android/os/storage/OnObbStateChangeListener.java
index a2d0a56..950195b 100644
--- a/core/java/android/os/storage/OnObbStateChangeListener.java
+++ b/core/java/android/os/storage/OnObbStateChangeListener.java
@@ -17,15 +17,69 @@
package android.os.storage;
/**
- * Used for receiving notifications from {@link StorageManager}.
+ * Used for receiving notifications from {@link StorageManager} about OBB file
+ * states.
*/
public abstract class OnObbStateChangeListener {
+
+ /**
+ * The OBB container is now mounted and ready for use. Returned in status
+ * messages from calls made via {@link StorageManager}
+ */
+ public static final int MOUNTED = 1;
+
+ /**
+ * The OBB container is now unmounted and not usable. Returned in status
+ * messages from calls made via {@link StorageManager}
+ */
+ public static final int UNMOUNTED = 2;
+
+ /**
+ * There was an internal system error encountered while trying to mount the
+ * OBB. Returned in status messages from calls made via
+ * {@link StorageManager}
+ */
+ public static final int ERROR_INTERNAL = 20;
+
+ /**
+ * The OBB could not be mounted by the system. Returned in status messages
+ * from calls made via {@link StorageManager}
+ */
+ public static final int ERROR_COULD_NOT_MOUNT = 21;
+
+ /**
+ * The OBB could not be unmounted. This most likely indicates that a file is
+ * in use on the OBB. Returned in status messages from calls made via
+ * {@link StorageManager}
+ */
+ public static final int ERROR_COULD_NOT_UNMOUNT = 22;
+
+ /**
+ * A call was made to unmount the OBB when it was not mounted. Returned in
+ * status messages from calls made via {@link StorageManager}
+ */
+ public static final int ERROR_NOT_MOUNTED = 23;
+
+ /**
+ * The OBB has already been mounted. Returned in status messages from calls
+ * made via {@link StorageManager}
+ */
+ public static final int ERROR_ALREADY_MOUNTED = 24;
+
+ /**
+ * The current application does not have permission to use this OBB because
+ * the OBB indicates it's owned by a different package or the key used to
+ * open it is incorrect. Returned in status messages from calls made via
+ * {@link StorageManager}
+ */
+ public static final int ERROR_PERMISSION_DENIED = 25;
+
/**
* Called when an OBB has changed states.
*
* @param path path to the OBB file the state change has happened on
* @param state the current state of the OBB
*/
- public void onObbStateChange(String path, String state) {
+ public void onObbStateChange(String path, int state) {
}
}
diff --git a/core/java/android/os/storage/StorageManager.java b/core/java/android/os/storage/StorageManager.java
index 2ebd049..73ac79f 100644
--- a/core/java/android/os/storage/StorageManager.java
+++ b/core/java/android/os/storage/StorageManager.java
@@ -22,12 +22,13 @@
import android.os.RemoteException;
import android.os.ServiceManager;
import android.util.Log;
+import android.util.Slog;
+import android.util.SparseArray;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.LinkedList;
import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* StorageManager is the interface to the systems storage service. The storage
@@ -69,7 +70,12 @@
/*
* List of our listeners
*/
- private ArrayList<ListenerDelegate> mListeners = new ArrayList<ListenerDelegate>();
+ private List<ListenerDelegate> mListeners = new ArrayList<ListenerDelegate>();
+
+ /*
+ * Next available nonce
+ */
+ final private AtomicInteger mNextNonce = new AtomicInteger(0);
private class MountServiceBinderListener extends IMountServiceListener.Stub {
public void onUsbMassStorageConnectionChanged(boolean available) {
@@ -93,57 +99,38 @@
private final ObbActionListener mObbActionListener = new ObbActionListener();
private class ObbActionListener extends IObbActionListener.Stub {
- private List<WeakReference<ObbListenerDelegate>> mListeners = new LinkedList<WeakReference<ObbListenerDelegate>>();
+ private SparseArray<ObbListenerDelegate> mListeners = new SparseArray<ObbListenerDelegate>();
@Override
- public void onObbResult(String filename, String status) throws RemoteException {
+ public void onObbResult(String filename, int nonce, int status) throws RemoteException {
+ final ObbListenerDelegate delegate;
synchronized (mListeners) {
- final Iterator<WeakReference<ObbListenerDelegate>> iter = mListeners.iterator();
- while (iter.hasNext()) {
- final WeakReference<ObbListenerDelegate> ref = iter.next();
-
- final ObbListenerDelegate delegate = (ref == null) ? null : ref.get();
- if (delegate == null) {
- iter.remove();
- continue;
- }
-
- delegate.sendObbStateChanged(filename, status);
+ delegate = mListeners.get(nonce);
+ if (delegate != null) {
+ mListeners.remove(nonce);
}
}
+
+ if (delegate != null) {
+ delegate.sendObbStateChanged(filename, status);
+ }
}
- public void addListener(OnObbStateChangeListener listener) {
- if (listener == null) {
- return;
- }
+ public int addListener(OnObbStateChangeListener listener) {
+ final ObbListenerDelegate delegate = new ObbListenerDelegate(listener);
synchronized (mListeners) {
- final Iterator<WeakReference<ObbListenerDelegate>> iter = mListeners.iterator();
- while (iter.hasNext()) {
- final WeakReference<ObbListenerDelegate> ref = iter.next();
-
- final ObbListenerDelegate delegate = (ref == null) ? null : ref.get();
- if (delegate == null) {
- iter.remove();
- continue;
- }
-
- /*
- * If we're already in the listeners, we don't need to be in
- * there again.
- */
- if (listener.equals(delegate.getListener())) {
- return;
- }
- }
-
- final ObbListenerDelegate delegate = new ObbListenerDelegate(listener);
- mListeners.add(new WeakReference<ObbListenerDelegate>(delegate));
+ mListeners.put(delegate.nonce, delegate);
}
+
+ return delegate.nonce;
}
}
+ private int getNextNonce() {
+ return mNextNonce.getAndIncrement();
+ }
+
/**
* Private class containing sender and receiver code for StorageEvents.
*/
@@ -151,7 +138,10 @@
private final WeakReference<OnObbStateChangeListener> mObbEventListenerRef;
private final Handler mHandler;
+ private final int nonce;
+
ObbListenerDelegate(OnObbStateChangeListener listener) {
+ nonce = getNextNonce();
mObbEventListenerRef = new WeakReference<OnObbStateChangeListener>(listener);
mHandler = new Handler(mTgtLooper) {
@Override
@@ -180,7 +170,7 @@
return mObbEventListenerRef.get();
}
- void sendObbStateChanged(String path, String state) {
+ void sendObbStateChanged(String path, int state) {
ObbStateChangedStorageEvent e = new ObbStateChangedStorageEvent(path, state);
mHandler.sendMessage(e.getMessage());
}
@@ -191,9 +181,10 @@
*/
private class ObbStateChangedStorageEvent extends StorageEvent {
public final String path;
- public final String state;
- public ObbStateChangedStorageEvent(String path, String state) {
+ public final int state;
+
+ public ObbStateChangedStorageEvent(String path, int state) {
super(EVENT_OBB_STATE_CHANGED);
this.path = path;
this.state = state;
@@ -420,10 +411,8 @@
* <p>
* The OBB will remain mounted for as long as the StorageManager reference
* is held by the application. As soon as this reference is lost, the OBBs
- * in use will be unmounted. The {@link OnObbStateChangeListener} registered with
- * this call will receive all further OBB-related events until it goes out
- * of scope. If the caller is not interested in whether the call succeeds,
- * the <code>listener</code> may be specified as <code>null</code>.
+ * in use will be unmounted. The {@link OnObbStateChangeListener} registered
+ * with this call will receive the success or failure of this operation.
* <p>
* <em>Note:</em> you can only mount OBB files for which the OBB tag on the
* file matches a package ID that is owned by the calling program's UID.
@@ -433,12 +422,21 @@
* @param filename the path to the OBB file
* @param key secret used to encrypt the OBB; may be <code>null</code> if no
* encryption was used on the OBB.
+ * @param listener will receive the success or failure of the operation
* @return whether the mount call was successfully queued or not
*/
public boolean mountObb(String filename, String key, OnObbStateChangeListener listener) {
+ if (filename == null) {
+ throw new IllegalArgumentException("filename cannot be null");
+ }
+
+ if (listener == null) {
+ throw new IllegalArgumentException("listener cannot be null");
+ }
+
try {
- mObbActionListener.addListener(listener);
- mMountService.mountObb(filename, key, mObbActionListener);
+ final int nonce = mObbActionListener.addListener(listener);
+ mMountService.mountObb(filename, key, mObbActionListener, nonce);
return true;
} catch (RemoteException e) {
Log.e(TAG, "Failed to mount OBB", e);
@@ -452,10 +450,8 @@
* <code>force</code> flag is true, it will kill any application needed to
* unmount the given OBB (even the calling application).
* <p>
- * The {@link OnObbStateChangeListener} registered with this call will receive all
- * further OBB-related events until it goes out of scope. If the caller is
- * not interested in whether the call succeeded, the listener may be
- * specified as <code>null</code>.
+ * The {@link OnObbStateChangeListener} registered with this call will
+ * receive the success or failure of this operation.
* <p>
* <em>Note:</em> you can only mount OBB files for which the OBB tag on the
* file matches a package ID that is owned by the calling program's UID.
@@ -466,12 +462,21 @@
* @param filename path to the OBB file
* @param force whether to kill any programs using this in order to unmount
* it
+ * @param listener will receive the success or failure of the operation
* @return whether the unmount call was successfully queued or not
*/
public boolean unmountObb(String filename, boolean force, OnObbStateChangeListener listener) {
+ if (filename == null) {
+ throw new IllegalArgumentException("filename cannot be null");
+ }
+
+ if (listener == null) {
+ throw new IllegalArgumentException("listener cannot be null");
+ }
+
try {
- mObbActionListener.addListener(listener);
- mMountService.unmountObb(filename, force, mObbActionListener);
+ final int nonce = mObbActionListener.addListener(listener);
+ mMountService.unmountObb(filename, force, mObbActionListener, nonce);
return true;
} catch (RemoteException e) {
Log.e(TAG, "Failed to mount OBB", e);
@@ -486,7 +491,11 @@
* @param filename path to OBB image
* @return true if OBB is mounted; false if not mounted or on error
*/
- public boolean isObbMounted(String filename) throws IllegalArgumentException {
+ public boolean isObbMounted(String filename) {
+ if (filename == null) {
+ throw new IllegalArgumentException("filename cannot be null");
+ }
+
try {
return mMountService.isObbMounted(filename);
} catch (RemoteException e) {
@@ -506,12 +515,14 @@
* not mounted or exception encountered trying to read status
*/
public String getMountedObbPath(String filename) {
+ if (filename == null) {
+ throw new IllegalArgumentException("filename cannot be null");
+ }
+
try {
return mMountService.getMountedObbPath(filename);
} catch (RemoteException e) {
Log.e(TAG, "Failed to find mounted path for OBB", e);
- } catch (IllegalArgumentException e) {
- Log.d(TAG, "Couldn't read OBB file", e);
}
return null;
diff --git a/core/tests/coretests/src/com/android/server/MountServiceTests.java b/core/tests/coretests/src/com/android/server/MountServiceTests.java
index 83e9d18..1f8c92e 100644
--- a/core/tests/coretests/src/com/android/server/MountServiceTests.java
+++ b/core/tests/coretests/src/com/android/server/MountServiceTests.java
@@ -21,7 +21,6 @@
import android.content.Context;
import android.content.res.Resources;
import android.content.res.Resources.NotFoundException;
-import android.os.Environment;
import android.os.FileUtils;
import android.os.storage.OnObbStateChangeListener;
import android.os.storage.StorageManager;
@@ -57,17 +56,15 @@
}
}
- private interface CompletableTask {
- public boolean isDone();
- }
+ private static class ObbObserver extends OnObbStateChangeListener {
+ private String path;
- private static class ObbObserver extends OnObbStateChangeListener implements CompletableTask {
- public String path;
- public String state;
+ public int state = -1;
boolean done = false;
@Override
- public void onObbStateChange(String path, String state) {
+ public void onObbStateChange(String path, int state) {
+ Log.d(TAG, "Received message. path=" + path + ", state=" + state);
synchronized (this) {
this.path = path;
this.state = state;
@@ -76,32 +73,43 @@
}
}
+ public String getPath() {
+ assertTrue("Expected ObbObserver to have received a state change.", done);
+ return path;
+ }
+
+ public int getState() {
+ assertTrue("Expected ObbObserver to have received a state change.", done);
+ return state;
+ }
+
public void reset() {
this.path = null;
- this.state = null;
+ this.state = -1;
done = false;
}
public boolean isDone() {
return done;
}
- }
- private boolean waitForCompletion(CompletableTask task) {
- long waitTime = 0;
- synchronized (task) {
- while (!task.isDone() && waitTime < MAX_WAIT_TIME) {
- try {
- task.wait(WAIT_TIME_INCR);
- waitTime += WAIT_TIME_INCR;
- } catch (InterruptedException e) {
- Log.i(TAG, "Interrupted during sleep", e);
+ public boolean waitForCompletion() {
+ long waitTime = 0;
+ synchronized (this) {
+ while (!isDone() && waitTime < MAX_WAIT_TIME) {
+ try {
+ wait(WAIT_TIME_INCR);
+ waitTime += WAIT_TIME_INCR;
+ } catch (InterruptedException e) {
+ Log.i(TAG, "Interrupted during sleep", e);
+ }
}
}
- }
- return task.isDone();
+ return isDone();
+ }
}
+
private File getFilePath(String name) {
final File filesDir = mContext.getFilesDir();
final File outFile = new File(filesDir, name);
@@ -128,23 +136,52 @@
}
private void mountObb(StorageManager sm, final int resource, final File file,
- String expectedState) {
+ int expectedState) {
copyRawToFile(resource, file);
- ObbObserver observer = new ObbObserver();
+ final ObbObserver observer = new ObbObserver();
assertTrue("mountObb call on " + file.getPath() + " should succeed",
sm.mountObb(file.getPath(), null, observer));
assertTrue("Mount should have completed",
- waitForCompletion(observer));
+ observer.waitForCompletion());
+
+ if (expectedState == OnObbStateChangeListener.MOUNTED) {
+ assertTrue("OBB should be mounted", sm.isObbMounted(file.getPath()));
+ }
assertEquals("Actual file and resolved file should be the same",
- file.getPath(), observer.path);
+ file.getPath(), observer.getPath());
- assertEquals(expectedState, observer.state);
+ assertEquals(expectedState, observer.getState());
}
- private String checkMountedPath(StorageManager sm, File file) {
+ private ObbObserver mountObbWithoutWait(final StorageManager sm, final int resource,
+ final File file) {
+ copyRawToFile(resource, file);
+
+ final ObbObserver observer = new ObbObserver();
+ assertTrue("mountObb call on " + file.getPath() + " should succeed", sm.mountObb(file
+ .getPath(), null, observer));
+
+ return observer;
+ }
+
+ private void waitForObbActionCompletion(final StorageManager sm, final File file,
+ final ObbObserver observer, int expectedState, boolean checkPath) {
+ assertTrue("Mount should have completed", observer.waitForCompletion());
+
+ assertTrue("OBB should be mounted", sm.isObbMounted(file.getPath()));
+
+ if (checkPath) {
+ assertEquals("Actual file and resolved file should be the same", file.getPath(),
+ observer.getPath());
+ }
+
+ assertEquals(expectedState, observer.getState());
+ }
+
+ private String checkMountedPath(final StorageManager sm, final File file) {
final String mountPath = sm.getMountedObbPath(file.getPath());
assertStartsWith("Path should be in " + OBB_MOUNT_PREFIX,
OBB_MOUNT_PREFIX,
@@ -152,13 +189,21 @@
return mountPath;
}
- private void unmountObb(StorageManager sm, final File outFile) {
- ObbObserver observer = new ObbObserver();
+ private void unmountObb(final StorageManager sm, final File file, int expectedState) {
+ final ObbObserver observer = new ObbObserver();
+
assertTrue("unmountObb call on test1.obb should succeed",
- sm.unmountObb(outFile.getPath(), false, observer));
+ sm.unmountObb(file.getPath(),
+ false, observer));
assertTrue("Unmount should have completed",
- waitForCompletion(observer));
+ observer.waitForCompletion());
+
+ assertEquals(expectedState, observer.getState());
+
+ if (expectedState == OnObbStateChangeListener.UNMOUNTED) {
+ assertFalse("OBB should not be mounted", sm.isObbMounted(file.getPath()));
+ }
}
@LargeTest
@@ -167,7 +212,9 @@
final File outFile = getFilePath("test1.obb");
- mountObb(sm, R.raw.test1, outFile, Environment.MEDIA_MOUNTED);
+ mountObb(sm, R.raw.test1, outFile, OnObbStateChangeListener.MOUNTED);
+
+ mountObb(sm, R.raw.test1, outFile, OnObbStateChangeListener.ERROR_ALREADY_MOUNTED);
final String mountPath = checkMountedPath(sm, outFile);
final File mountDir = new File(mountPath);
@@ -175,7 +222,7 @@
assertTrue("OBB mounted path should be a directory",
mountDir.isDirectory());
- unmountObb(sm, outFile);
+ unmountObb(sm, outFile, OnObbStateChangeListener.UNMOUNTED);
}
@LargeTest
@@ -184,7 +231,7 @@
final File outFile = getFilePath("test1_nosig.obb");
- mountObb(sm, R.raw.test1_nosig, outFile, Environment.MEDIA_BAD_REMOVAL);
+ mountObb(sm, R.raw.test1_nosig, outFile, OnObbStateChangeListener.ERROR_INTERNAL);
assertFalse("OBB should not be mounted",
sm.isObbMounted(outFile.getPath()));
@@ -199,7 +246,8 @@
final File outFile = getFilePath("test1_wrongpackage.obb");
- mountObb(sm, R.raw.test1_wrongpackage, outFile, Environment.MEDIA_BAD_REMOVAL);
+ mountObb(sm, R.raw.test1_wrongpackage, outFile,
+ OnObbStateChangeListener.ERROR_PERMISSION_DENIED);
assertFalse("OBB should not be mounted",
sm.isObbMounted(outFile.getPath()));
@@ -207,4 +255,31 @@
assertNull("OBB's mounted path should be null",
sm.getMountedObbPath(outFile.getPath()));
}
+
+ @LargeTest
+ public void testMountAndUnmountTwoObbs() {
+ StorageManager sm = getStorageManager();
+
+ final File file1 = getFilePath("test1.obb");
+ final File file2 = getFilePath("test2.obb");
+
+ ObbObserver oo1 = mountObbWithoutWait(sm, R.raw.test1, file1);
+ ObbObserver oo2 = mountObbWithoutWait(sm, R.raw.test1, file2);
+
+ Log.d(TAG, "Waiting for OBB #1 to complete mount");
+ waitForObbActionCompletion(sm, file1, oo1, OnObbStateChangeListener.MOUNTED, false);
+ Log.d(TAG, "Waiting for OBB #2 to complete mount");
+ waitForObbActionCompletion(sm, file2, oo2, OnObbStateChangeListener.MOUNTED, false);
+
+ final String mountPath1 = checkMountedPath(sm, file1);
+ final File mountDir1 = new File(mountPath1);
+ assertTrue("OBB mounted path should be a directory", mountDir1.isDirectory());
+
+ final String mountPath2 = checkMountedPath(sm, file2);
+ final File mountDir2 = new File(mountPath2);
+ assertTrue("OBB mounted path should be a directory", mountDir2.isDirectory());
+
+ unmountObb(sm, file1, OnObbStateChangeListener.UNMOUNTED);
+ unmountObb(sm, file2, OnObbStateChangeListener.UNMOUNTED);
+ }
}
diff --git a/include/storage/IMountService.h b/include/storage/IMountService.h
index 436fc38..51f9aeb 100644
--- a/include/storage/IMountService.h
+++ b/include/storage/IMountService.h
@@ -61,9 +61,9 @@
virtual void shutdown(const sp<IMountShutdownObserver>& observer) = 0;
virtual void finishMediaUpdate() = 0;
virtual void mountObb(const String16& filename, const String16& key,
- const sp<IObbActionListener>& token) = 0;
+ const sp<IObbActionListener>& token, const int32_t nonce) = 0;
virtual void unmountObb(const String16& filename, const bool force,
- const sp<IObbActionListener>& token) = 0;
+ const sp<IObbActionListener>& token, const int32_t nonce) = 0;
virtual bool isObbMounted(const String16& filename) = 0;
virtual bool getMountedObbPath(const String16& filename, String16& path) = 0;
};
diff --git a/include/storage/IObbActionListener.h b/include/storage/IObbActionListener.h
index 1bedcc6..d78273c 100644
--- a/include/storage/IObbActionListener.h
+++ b/include/storage/IObbActionListener.h
@@ -29,7 +29,7 @@
public:
DECLARE_META_INTERFACE(ObbActionListener);
- virtual void onObbResult(const String16& filename, const String16& status) = 0;
+ virtual void onObbResult(const String16& filename, const int32_t nonce, const int32_t state) = 0;
};
// ----------------------------------------------------------------------------
diff --git a/libs/storage/IMountService.cpp b/libs/storage/IMountService.cpp
index 3ad9319..f36e2a3 100644
--- a/libs/storage/IMountService.cpp
+++ b/libs/storage/IMountService.cpp
@@ -430,13 +430,14 @@
}
void mountObb(const String16& filename, const String16& key,
- const sp<IObbActionListener>& token)
+ const sp<IObbActionListener>& token, int32_t nonce)
{
Parcel data, reply;
data.writeInterfaceToken(IMountService::getInterfaceDescriptor());
data.writeString16(filename);
data.writeString16(key);
data.writeStrongBinder(token->asBinder());
+ data.writeInt32(nonce);
if (remote()->transact(TRANSACTION_mountObb, data, &reply) != NO_ERROR) {
LOGD("mountObb could not contact remote\n");
return;
@@ -448,7 +449,7 @@
}
}
- void unmountObb(const String16& filename, const bool force, const sp<IObbActionListener>& token)
+ void unmountObb(const String16& filename, const bool force)
{
Parcel data, reply;
data.writeInterfaceToken(IMountService::getInterfaceDescriptor());
diff --git a/libs/storage/IObbActionListener.cpp b/libs/storage/IObbActionListener.cpp
index 5bfece7..eaa211e 100644
--- a/libs/storage/IObbActionListener.cpp
+++ b/libs/storage/IObbActionListener.cpp
@@ -30,7 +30,7 @@
: BpInterface<IObbActionListener>(impl)
{ }
- virtual void onObbResult(const String16& filename, const String16& status) { }
+ virtual void onObbResult(const String16& filename, const int32_t nonce, const int32_t state) { }
};
IMPLEMENT_META_INTERFACE(ObbActionListener, "IObbActionListener");
@@ -44,8 +44,9 @@
case TRANSACTION_onObbResult: {
CHECK_INTERFACE(IObbActionListener, data, reply);
String16 filename = data.readString16();
- String16 state = data.readString16();
- onObbResult(filename, state);
+ int32_t nonce = data.readInt32();
+ int32_t state = data.readInt32();
+ onObbResult(filename, nonce, state);
reply->writeNoException();
return NO_ERROR;
} break;
diff --git a/native/android/storage_manager.cpp b/native/android/storage_manager.cpp
index 2f20641..a4233e7 100644
--- a/native/android/storage_manager.cpp
+++ b/native/android/storage_manager.cpp
@@ -21,10 +21,13 @@
#include <binder/Binder.h>
#include <binder/IServiceManager.h>
+#include <utils/Atomic.h>
#include <utils/Log.h>
#include <utils/RefBase.h>
#include <utils/String8.h>
#include <utils/String16.h>
+#include <utils/Vector.h>
+#include <utils/threads.h>
using namespace android;
@@ -38,20 +41,46 @@
mStorageManager(mgr)
{}
- virtual void onObbResult(const android::String16& filename, const android::String16& state);
+ virtual void onObbResult(const android::String16& filename, const int32_t nonce,
+ const int32_t state);
+};
+
+class ObbCallback {
+public:
+ ObbCallback(int32_t _nonce, AStorageManager_obbCallbackFunc _cb, void* _data)
+ : nonce(_nonce)
+ , cb(_cb)
+ , data(_data)
+ {}
+
+ int32_t nonce;
+ AStorageManager_obbCallbackFunc cb;
+ void* data;
};
struct AStorageManager : public RefBase {
protected:
- AStorageManager_obbCallbackFunc mObbCallback;
- void* mObbCallbackData;
+ Mutex mCallbackLock;
+ Vector<ObbCallback*> mCallbacks;
+ volatile int32_t mNextNonce;
sp<ObbActionListener> mObbActionListener;
sp<IMountService> mMountService;
+ int32_t getNextNonce() {
+ return android_atomic_inc(&mNextNonce);
+ }
+
+ ObbCallback* registerObbCallback(AStorageManager_obbCallbackFunc func, void* data) {
+ ObbCallback* cb = new ObbCallback(getNextNonce(), func, data);
+ {
+ AutoMutex _l(mCallbackLock);
+ mCallbacks.push(cb);
+ }
+ return cb;
+ }
+
public:
AStorageManager()
- : mObbCallback(NULL)
- , mObbCallbackData(NULL)
{
}
@@ -73,26 +102,40 @@
return true;
}
- void setObbCallback(AStorageManager_obbCallbackFunc cb, void* data) {
- mObbCallback = cb;
- mObbCallbackData = data;
- }
+ void fireCallback(const char* filename, const int32_t nonce, const int32_t state) {
+ ObbCallback* target = NULL;
+ {
+ AutoMutex _l(mCallbackLock);
+ int N = mCallbacks.size();
+ for (int i = 0; i < N; i++) {
+ ObbCallback* cb = mCallbacks.editItemAt(i);
+ if (cb->nonce == nonce) {
+ target = cb;
+ mCallbacks.removeAt(i);
+ break;
+ }
+ }
+ }
- void fireCallback(const char* filename, const char* state) {
- if (mObbCallback != NULL) {
- mObbCallback(filename, state, mObbCallbackData);
+ if (target != NULL) {
+ target->cb(filename, state, target->data);
+ delete target;
+ } else {
+ LOGI("Didn't find the callback handler for: %s\n", filename);
}
}
- void mountObb(const char* filename, const char* key) {
+ void mountObb(const char* filename, const char* key, AStorageManager_obbCallbackFunc func, void* data) {
+ ObbCallback* cb = registerObbCallback(func, data);
String16 filename16(filename);
String16 key16(key);
- mMountService->mountObb(filename16, key16, mObbActionListener);
+ mMountService->mountObb(filename16, key16, mObbActionListener, cb->nonce);
}
- void unmountObb(const char* filename, const bool force) {
+ void unmountObb(const char* filename, const bool force, AStorageManager_obbCallbackFunc func, void* data) {
+ ObbCallback* cb = registerObbCallback(func, data);
String16 filename16(filename);
- mMountService->unmountObb(filename16, force, mObbActionListener);
+ mMountService->unmountObb(filename16, force, mObbActionListener, cb->nonce);
}
int isObbMounted(const char* filename) {
@@ -111,8 +154,8 @@
}
};
-void ObbActionListener::onObbResult(const android::String16& filename, const android::String16& state) {
- mStorageManager->fireCallback(String8(filename).string(), String8(state).string());
+void ObbActionListener::onObbResult(const android::String16& filename, const int32_t nonce, const int32_t state) {
+ mStorageManager->fireCallback(String8(filename).string(), nonce, state);
}
@@ -131,16 +174,14 @@
}
}
-void AStorageManager_setObbCallback(AStorageManager* mgr, AStorageManager_obbCallbackFunc cb, void* data) {
- mgr->setObbCallback(cb, data);
+void AStorageManager_mountObb(AStorageManager* mgr, const char* filename, const char* key,
+ AStorageManager_obbCallbackFunc cb, void* data) {
+ mgr->mountObb(filename, key, cb, data);
}
-void AStorageManager_mountObb(AStorageManager* mgr, const char* filename, const char* key) {
- mgr->mountObb(filename, key);
-}
-
-void AStorageManager_unmountObb(AStorageManager* mgr, const char* filename, const int force) {
- mgr->unmountObb(filename, force != 0);
+void AStorageManager_unmountObb(AStorageManager* mgr, const char* filename, const int force,
+ AStorageManager_obbCallbackFunc cb, void* data) {
+ mgr->unmountObb(filename, force != 0, cb, data);
}
int AStorageManager_isObbMounted(AStorageManager* mgr, const char* filename) {
diff --git a/native/include/android/storage_manager.h b/native/include/android/storage_manager.h
index 6f925c1..c202693 100644
--- a/native/include/android/storage_manager.h
+++ b/native/include/android/storage_manager.h
@@ -18,6 +18,8 @@
#ifndef ANDROID_STORAGE_MANAGER_H
#define ANDROID_STORAGE_MANAGER_H
+#include <stdint.h>
+
#ifdef __cplusplus
extern "C" {
#endif
@@ -25,6 +27,60 @@
struct AStorageManager;
typedef struct AStorageManager AStorageManager;
+enum {
+ /*
+ * The OBB container is now mounted and ready for use. Can be returned
+ * as the status for callbacks made during asynchronous OBB actions.
+ */
+ AOBB_STATE_MOUNTED = 1,
+
+ /*
+ * The OBB container is now unmounted and not usable. Can be returned
+ * as the status for callbacks made during asynchronous OBB actions.
+ */
+ AOBB_STATE_UNMOUNTED = 2,
+
+ /*
+ * There was an internal system error encountered while trying to
+ * mount the OBB. Can be returned as the status for callbacks made
+ * during asynchronous OBB actions.
+ */
+ AOBB_STATE_ERROR_INTERNAL = 20,
+
+ /*
+ * The OBB could not be mounted by the system. Can be returned as the
+ * status for callbacks made during asynchronous OBB actions.
+ */
+ AOBB_STATE_ERROR_COULD_NOT_MOUNT = 21,
+
+ /*
+ * The OBB could not be unmounted. This most likely indicates that a
+ * file is in use on the OBB. Can be returned as the status for
+ * callbacks made during asynchronous OBB actions.
+ */
+ AOBB_STATE_ERROR_COULD_NOT_UNMOUNT = 22,
+
+ /*
+ * A call was made to unmount the OBB when it was not mounted. Can be
+ * returned as the status for callbacks made during asynchronous OBB
+ * actions.
+ */
+ AOBB_STATE_ERROR_NOT_MOUNTED = 23,
+
+ /*
+ * The OBB has already been mounted. Can be returned as the status for
+ * callbacks made during asynchronous OBB actions.
+ */
+ AOBB_STATE_ERROR_ALREADY_MOUNTED = 24,
+
+ /*
+ * The current application does not have permission to use this OBB
+ * because the OBB indicates it's owned by a different package or the
+ * key used to open it is incorrect. Can be returned as the status for
+ * callbacks made during asynchronous OBB actions.
+ */
+ AOBB_STATE_ERROR_PERMISSION_DENIED = 25,
+};
/**
* Obtains a new instance of AStorageManager.
@@ -39,22 +95,19 @@
/**
* Callback function for asynchronous calls made on OBB files.
*/
-typedef void (*AStorageManager_obbCallbackFunc)(const char* filename, const char* state, void* data);
-
-/**
- * Callback to call when requested asynchronous OBB operation is complete.
- */
-void AStorageManager_setObbCallback(AStorageManager* mgr, AStorageManager_obbCallbackFunc cb, void* data);
+typedef void (*AStorageManager_obbCallbackFunc)(const char* filename, const int32_t state, void* data);
/**
* Attempts to mount an OBB file. This is an asynchronous operation.
*/
-void AStorageManager_mountObb(AStorageManager* mgr, const char* filename, const char* key);
+void AStorageManager_mountObb(AStorageManager* mgr, const char* filename, const char* key,
+ AStorageManager_obbCallbackFunc cb, void* data);
/**
* Attempts to unmount an OBB file. This is an asynchronous operation.
*/
-void AStorageManager_unmountObb(AStorageManager* mgr, const char* filename, const int force);
+void AStorageManager_unmountObb(AStorageManager* mgr, const char* filename, const int force,
+ AStorageManager_obbCallbackFunc cb, void* data);
/**
* Check whether an OBB is mounted.
diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java
index 3b2d836..775f5c8 100644
--- a/services/java/com/android/server/MountService.java
+++ b/services/java/com/android/server/MountService.java
@@ -44,6 +44,7 @@
import android.os.storage.IMountServiceListener;
import android.os.storage.IMountShutdownObserver;
import android.os.storage.IObbActionListener;
+import android.os.storage.OnObbStateChangeListener;
import android.os.storage.StorageResultCode;
import android.security.MessageDigest;
import android.util.Slog;
@@ -53,7 +54,6 @@
import java.io.PrintWriter;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -161,25 +161,25 @@
final private Map<String, ObbState> mObbPathToStateMap = new HashMap<String, ObbState>();
class ObbState implements IBinder.DeathRecipient {
- public ObbState(String filename, IObbActionListener token, int callerUid)
+ public ObbState(String filename, int callerUid, IObbActionListener token, int nonce)
throws RemoteException {
this.filename = filename;
- this.token = token;
this.callerUid = callerUid;
- mounted = false;
+ this.token = token;
+ this.nonce = nonce;
}
// OBB source filename
- final String filename;
-
- // Token of remote Binder caller
- final IObbActionListener token;
+ String filename;
// Binder.callingUid()
final public int callerUid;
- // Whether this is mounted currently.
- boolean mounted;
+ // Token of remote Binder caller
+ final IObbActionListener token;
+
+ // Identifier to pass back to the token
+ final int nonce;
public IBinder getBinder() {
return token.asBinder();
@@ -208,8 +208,6 @@
sb.append(token.toString());
sb.append(",callerUid=");
sb.append(callerUid);
- sb.append(",mounted=");
- sb.append(mounted);
sb.append('}');
return sb.toString();
}
@@ -223,6 +221,7 @@
private static final int OBB_MCS_BOUND = 2;
private static final int OBB_MCS_UNBIND = 3;
private static final int OBB_MCS_RECONNECT = 4;
+ private static final int OBB_FLUSH_MOUNT_STATE = 5;
/*
* Default Container Service information
@@ -500,40 +499,23 @@
Slog.w(TAG, String.format("Duplicate state transition (%s -> %s)", mLegacyState, state));
return;
}
- // Update state on PackageManager
+
if (Environment.MEDIA_UNMOUNTED.equals(state)) {
+ // Tell the package manager the media is gone.
mPms.updateExternalMediaStatus(false, false);
+
+ /*
+ * Some OBBs might have been unmounted when this volume was
+ * unmounted, so send a message to the handler to let it know to
+ * remove those from the list of mounted OBBS.
+ */
+ mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_FLUSH_MOUNT_STATE,
+ path));
} else if (Environment.MEDIA_MOUNTED.equals(state)) {
+ // Tell the package manager the media is available for use.
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;
@@ -1530,6 +1512,10 @@
}
public String getMountedObbPath(String filename) {
+ if (filename == null) {
+ throw new IllegalArgumentException("filename cannot be null");
+ }
+
waitForReady();
warnOnNotMounted();
@@ -1552,164 +1538,98 @@
}
public boolean isObbMounted(String filename) {
- synchronized (mObbMounts) {
- final ObbState obbState = mObbPathToStateMap.get(filename);
- if (obbState != null) {
- synchronized (obbState) {
- return obbState.mounted;
- }
- }
+ if (filename == null) {
+ throw new IllegalArgumentException("filename cannot be null");
}
- return false;
+
+ synchronized (mObbMounts) {
+ return mObbPathToStateMap.containsKey(filename);
+ }
}
- public void mountObb(String filename, String key, IObbActionListener token)
+ public void mountObb(String filename, String key, IObbActionListener token, int nonce)
throws RemoteException {
- waitForReady();
- warnOnNotMounted();
-
if (filename == null) {
throw new IllegalArgumentException("filename cannot be null");
- } else if (token == null) {
+ }
+
+ if (token == null) {
throw new IllegalArgumentException("token cannot be null");
}
- final ObbState obbState;
-
- synchronized (mObbMounts) {
- if (isObbMounted(filename)) {
- 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();
- obbState = new ObbState(filename, token, callerUid);
- addObbState(obbState);
- }
-
- String hashedKey = null;
- if (key != null) {
- final MessageDigest md;
- try {
- md = MessageDigest.getInstance("MD5");
- } catch (NoSuchAlgorithmException e) {
- Slog.e(TAG, "Could not load MD5 algorithm", e);
- try {
- token.onObbResult(filename, Environment.MEDIA_UNMOUNTED);
- } catch (RemoteException e1) {
- Slog.d(TAG, "Could not send unmount notification for: " + filename);
- }
- return;
- }
-
- hashedKey = HexDump.toHexString(md.digest(key.getBytes()));
- }
-
- ObbAction action = new MountObbAction(obbState, hashedKey);
+ final int callerUid = Binder.getCallingUid();
+ final ObbState obbState = new ObbState(filename, callerUid, token, nonce);
+ final ObbAction action = new MountObbAction(obbState, key);
mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
if (DEBUG_OBB)
Slog.i(TAG, "Send to OBB handler: " + action.toString());
}
- public void unmountObb(String filename, boolean force, IObbActionListener token) {
+ public void unmountObb(String filename, boolean force, IObbActionListener token, int nonce)
+ throws RemoteException {
if (filename == null) {
throw new IllegalArgumentException("filename cannot be null");
- } else if (token == null) {
- throw new IllegalArgumentException("token cannot be null");
}
- final ObbState obbState;
-
- synchronized (mObbMounts) {
- if (!isObbMounted(filename)) {
- 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) {
- throw new SecurityException("caller UID does not match original mount caller UID");
- } else if (!token.asBinder().equals(obbState.getBinder())) {
- throw new SecurityException("caller does not match original mount caller");
- }
- }
-
- ObbAction action = new UnmountObbAction(obbState, force);
+ final int callerUid = Binder.getCallingUid();
+ final ObbState obbState = new ObbState(filename, callerUid, token, nonce);
+ final ObbAction action = new UnmountObbAction(obbState, force);
mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
if (DEBUG_OBB)
Slog.i(TAG, "Send to OBB handler: " + action.toString());
}
- private void addObbState(ObbState obbState) throws RemoteException {
- synchronized (mObbMounts) {
- final IBinder binder = obbState.getBinder();
- List<ObbState> obbStates = mObbMounts.get(binder);
- final boolean unique;
+ private void addObbStateLocked(ObbState obbState) throws RemoteException {
+ final IBinder binder = obbState.getBinder();
+ List<ObbState> obbStates = mObbMounts.get(binder);
- if (obbStates == null) {
- obbStates = new ArrayList<ObbState>();
- mObbMounts.put(binder, obbStates);
- unique = true;
- } else {
- unique = obbStates.contains(obbState);
- }
-
- if (unique) {
- obbStates.add(obbState);
- try {
- obbState.link();
- } catch (RemoteException e) {
- /*
- * The binder died before we could link it, so clean up our
- * state and return failure.
- */
- obbStates.remove(obbState);
- if (obbStates.isEmpty()) {
- mObbMounts.remove(binder);
- }
-
- // Rethrow the error so mountObb can get it
- throw e;
+ if (obbStates == null) {
+ obbStates = new ArrayList<ObbState>();
+ mObbMounts.put(binder, obbStates);
+ } else {
+ for (final ObbState o : obbStates) {
+ if (o.filename.equals(obbState.filename)) {
+ throw new IllegalStateException("Attempt to add ObbState twice. "
+ + "This indicates an error in the MountService logic.");
}
}
-
- mObbPathToStateMap.put(obbState.filename, obbState);
}
+
+ obbStates.add(obbState);
+ try {
+ obbState.link();
+ } catch (RemoteException e) {
+ /*
+ * The binder died before we could link it, so clean up our state
+ * and return failure.
+ */
+ obbStates.remove(obbState);
+ if (obbStates.isEmpty()) {
+ mObbMounts.remove(binder);
+ }
+
+ // Rethrow the error so mountObb can get it
+ throw e;
+ }
+
+ mObbPathToStateMap.put(obbState.filename, obbState);
}
- private void removeObbState(ObbState obbState) {
- synchronized (mObbMounts) {
- final IBinder binder = obbState.getBinder();
- final List<ObbState> obbStates = mObbMounts.get(binder);
- if (obbStates != null) {
- if (obbStates.remove(obbState)) {
- obbState.unlink();
- }
- if (obbStates.isEmpty()) {
- mObbMounts.remove(binder);
- }
+ private void removeObbStateLocked(ObbState obbState) {
+ final IBinder binder = obbState.getBinder();
+ final List<ObbState> obbStates = mObbMounts.get(binder);
+ if (obbStates != null) {
+ if (obbStates.remove(obbState)) {
+ obbState.unlink();
}
-
- mObbPathToStateMap.remove(obbState.filename);
+ if (obbStates.isEmpty()) {
+ mObbMounts.remove(binder);
+ }
}
- }
- private void replaceObbState(ObbState oldObbState, ObbState newObbState) throws RemoteException {
- synchronized (mObbMounts) {
- removeObbState(oldObbState);
- addObbState(newObbState);
- }
+ mObbPathToStateMap.remove(obbState.filename);
}
private class ObbActionHandler extends Handler {
@@ -1808,6 +1728,47 @@
}
break;
}
+ case OBB_FLUSH_MOUNT_STATE: {
+ final String path = (String) msg.obj;
+
+ if (DEBUG_OBB)
+ Slog.i(TAG, "Flushing all OBB state for path " + 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) {
+ if (DEBUG_OBB)
+ Slog.i(TAG, "Removing state for " + obbState.filename);
+
+ removeObbStateLocked(obbState);
+
+ try {
+ obbState.token.onObbResult(obbState.filename, obbState.nonce,
+ OnObbStateChangeListener.UNMOUNTED);
+ } catch (RemoteException e) {
+ Slog.i(TAG, "Couldn't send unmount notification for OBB: "
+ + obbState.filename);
+ }
+ }
+ }
+ break;
+ }
}
}
@@ -1886,9 +1847,13 @@
return obbInfo;
}
- protected void sendNewStatusOrIgnore(String filename, String status) {
+ protected void sendNewStatusOrIgnore(int status) {
+ if (mObbState == null || mObbState.token == null) {
+ return;
+ }
+
try {
- mObbState.token.onObbResult(filename, status);
+ mObbState.token.onObbResult(mObbState.filename, mObbState.nonce, status);
} catch (RemoteException e) {
Slog.w(TAG, "MountServiceListener went away while calling onObbStateChanged");
}
@@ -1904,89 +1869,80 @@
}
public void handleExecute() throws IOException, RemoteException {
+ waitForReady();
+ warnOnNotMounted();
+
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)) {
- throw new IllegalArgumentException("Caller package does not match OBB file");
+ Slog.w(TAG, "Denied attempt to mount OBB " + obbInfo.filename
+ + " which is owned by " + obbInfo.packageName);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_PERMISSION_DENIED);
+ return;
}
- boolean mounted = false;
- int rc;
- synchronized (mObbState) {
- if (mObbState.mounted) {
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_MOUNTED);
+ final boolean isMounted;
+ synchronized (mObbMounts) {
+ isMounted = mObbPathToStateMap.containsKey(obbInfo.filename);
+ }
+ if (isMounted) {
+ Slog.w(TAG, "Attempt to mount OBB which is already mounted: " + obbInfo.filename);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_ALREADY_MOUNTED);
+ return;
+ }
+
+ /*
+ * The filename passed in might not be the canonical name, so just
+ * set the filename to the canonicalized version.
+ */
+ mObbState.filename = obbInfo.filename;
+
+ final String hashedKey;
+ if (mKey == null) {
+ hashedKey = "none";
+ } else {
+ final MessageDigest md;
+ try {
+ md = MessageDigest.getInstance("MD5");
+ } catch (NoSuchAlgorithmException e) {
+ Slog.e(TAG, "Could not load MD5 algorithm", e);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_PERMISSION_DENIED);
return;
}
- rc = StorageResultCode.OperationSucceeded;
- String cmd = String.format("obb mount %s %s %d", mObbState.filename,
- mKey != null ? mKey : "none",
- mObbState.callerUid);
- try {
- mConnector.doCommand(cmd);
- } catch (NativeDaemonConnectorException e) {
- int code = e.getCode();
- if (code != VoldResponseCode.OpFailedStorageBusy) {
- rc = StorageResultCode.OperationFailedInternalError;
- }
- }
+ hashedKey = HexDump.toHexString(md.digest(mKey.getBytes()));
+ }
- if (rc == StorageResultCode.OperationSucceeded) {
- mObbState.mounted = mounted = true;
+ int rc = StorageResultCode.OperationSucceeded;
+ String cmd = String.format("obb mount %s %s %d", mObbState.filename, hashedKey,
+ mObbState.callerUid);
+ try {
+ mConnector.doCommand(cmd);
+ } catch (NativeDaemonConnectorException e) {
+ int code = e.getCode();
+ if (code != VoldResponseCode.OpFailedStorageBusy) {
+ rc = StorageResultCode.OperationFailedInternalError;
}
}
- if (mounted) {
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_MOUNTED);
+ if (rc == StorageResultCode.OperationSucceeded) {
+ if (DEBUG_OBB)
+ Slog.d(TAG, "Successfully mounted OBB " + mObbState.filename);
+
+ synchronized (mObbMounts) {
+ addObbStateLocked(mObbState);
+ }
+
+ sendNewStatusOrIgnore(OnObbStateChangeListener.MOUNTED);
} else {
Slog.e(TAG, "Couldn't mount OBB file: " + rc);
- // We didn't succeed, so remove this from the mount-set.
- removeObbState(mObbState);
-
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_UNMOUNTED);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_COULD_NOT_MOUNT);
}
}
public void handleError() {
- removeObbState(mObbState);
-
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_INTERNAL);
}
@Override
@@ -1999,6 +1955,8 @@
sb.append(mObbState.callerUid);
sb.append(",token=");
sb.append(mObbState.token != null ? mObbState.token.toString() : "NULL");
+ sb.append(",binder=");
+ sb.append(mObbState.token != null ? mObbState.getBinder().toString() : "null");
sb.append('}');
return sb.toString();
}
@@ -2013,68 +1971,61 @@
}
public void handleExecute() throws IOException {
+ waitForReady();
+ warnOnNotMounted();
+
final ObbInfo obbInfo = getObbInfo();
- /*
- * If someone tried to trick us with some weird characters, rectify
- * it here.
- */
+ final ObbState obbState;
synchronized (mObbMounts) {
- if (!isObbMounted(obbInfo.filename)) {
- removeObbState(mObbState);
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_UNMOUNTED);
- return;
- }
+ obbState = mObbPathToStateMap.get(obbInfo.filename);
+ }
- if (!mObbState.filename.equals(obbInfo.filename)) {
- removeObbState(mObbState);
- mObbState = mObbPathToStateMap.get(obbInfo.filename);
+ if (obbState == null) {
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_NOT_MOUNTED);
+ return;
+ }
+
+ if (obbState.callerUid != mObbState.callerUid) {
+ Slog.w(TAG, "Permission denied attempting to unmount OBB " + obbInfo.filename
+ + " (owned by " + obbInfo.packageName + ")");
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_PERMISSION_DENIED);
+ return;
+ }
+
+ mObbState.filename = obbInfo.filename;
+
+ 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 if (code == VoldResponseCode.OpFailedStorageNotFound) {
+ // If it's not mounted then we've already won.
+ rc = StorageResultCode.OperationSucceeded;
+ } else {
+ rc = StorageResultCode.OperationFailedInternalError;
}
}
- boolean unmounted = false;
- synchronized (mObbState) {
- if (!mObbState.mounted) {
- sendNewStatusOrIgnore(obbInfo.filename, Environment.MEDIA_UNMOUNTED);
- return;
+ if (rc == StorageResultCode.OperationSucceeded) {
+ synchronized (mObbMounts) {
+ removeObbStateLocked(obbState);
}
- 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 if (code == VoldResponseCode.OpFailedStorageNotFound) {
- // If it's not mounted then we've already won.
- rc = StorageResultCode.OperationSucceeded;
- } else {
- rc = StorageResultCode.OperationFailedInternalError;
- }
- }
-
- if (rc == StorageResultCode.OperationSucceeded) {
- mObbState.mounted = false;
- unmounted = true;
- }
- }
-
- if (unmounted) {
- removeObbState(mObbState);
-
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_UNMOUNTED);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.UNMOUNTED);
} else {
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_MOUNTED);
+ Slog.w(TAG, "Could not mount OBB: " + mObbState.filename);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_COULD_NOT_UNMOUNT);
}
}
public void handleError() {
- removeObbState(mObbState);
-
- sendNewStatusOrIgnore(mObbState.filename, Environment.MEDIA_BAD_REMOVAL);
+ sendNewStatusOrIgnore(OnObbStateChangeListener.ERROR_INTERNAL);
}
@Override
@@ -2090,7 +2041,7 @@
sb.append(",token=");
sb.append(mObbState.token != null ? mObbState.token.toString() : "null");
sb.append(",binder=");
- sb.append(mObbState.getBinder().toString());
+ sb.append(mObbState.token != null ? mObbState.getBinder().toString() : "null");
sb.append('}');
return sb.toString();
}
@@ -2105,16 +2056,27 @@
return;
}
- pw.println(" mObbMounts:");
-
synchronized (mObbMounts) {
- final Collection<List<ObbState>> obbStateLists = mObbMounts.values();
+ pw.println(" mObbMounts:");
- for (final List<ObbState> obbStates : obbStateLists) {
+ final Iterator<Entry<IBinder, List<ObbState>>> binders = mObbMounts.entrySet().iterator();
+ while (binders.hasNext()) {
+ Entry<IBinder, List<ObbState>> e = binders.next();
+ pw.print(" Key="); pw.println(e.getKey().toString());
+ final List<ObbState> obbStates = e.getValue();
for (final ObbState obbState : obbStates) {
- pw.print(" "); pw.println(obbState.toString());
+ pw.print(" "); pw.println(obbState.toString());
}
}
+
+ pw.println("");
+ pw.println(" mObbPathToStateMap:");
+ final Iterator<Entry<String, ObbState>> maps = mObbPathToStateMap.entrySet().iterator();
+ while (maps.hasNext()) {
+ final Entry<String, ObbState> e = maps.next();
+ pw.print(" "); pw.print(e.getKey());
+ pw.print(" -> "); pw.println(e.getValue().toString());
+ }
}
}
}