Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use futures for binding and talking to the ExternalStorageService.

Browse Source 
The existing CountdownLatch logic had some corner cases where it would
continue waiting for the latch to count down, even when it was already
clear that the operation couldn't succeed (eg, a bind failed).

To fix this, as well as try and simplify some of this code, switch to
using futures, which have a nicely defined way of cancelling outstanding
operations.

Additionally, switch to using a dedicated handler thread for receiving
bind events for all users, not just demo users. This avoids having to
wait for a long time when the system_server main thread is busy.

Bug: 170284679
Test: atest AdoptableHostTest
      atest CtsScopedStorageHostTest
      atest android.multiuser.UserLifecycleTests#managedProfileUnlock
      atest android.multiuser.UserLifecycleTests#managedProfileUnlockAndLaunchApp_stopped
Change-Id: I1b7fde2f6ed1074b9c814d56808ab1614f25aead
This commit is contained in:
Martijn Coenen
2020-11-24 13:24:53 +01:00
parent ed7b77e88d
commit 9252dfb493
1 changed files with 105 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files

279
services/core/java/com/android/server/storage/StorageUserConnection.java
View File

@@ -23,6 +23,7 @@ import static android.service.storage.ExternalStorageService.FLAG_SESSION_TYPE_F
import static com.android.server.storage.StorageSessionController.ExternalStorageServiceException; import static com.android.server.storage.StorageSessionController.ExternalStorageServiceException;
import android.annotation.MainThread; import android.annotation.MainThread;
import android.annotation.NonNull;
import android.annotation.Nullable; import android.annotation.Nullable;
import android.content.ComponentName; import android.content.ComponentName;
import android.content.Context; import android.content.Context;
@@ -34,28 +35,27 @@ import android.os.IBinder;
import android.os.ParcelFileDescriptor; import android.os.ParcelFileDescriptor;
import android.os.ParcelableException; import android.os.ParcelableException;
import android.os.RemoteCallback; import android.os.RemoteCallback;
import android.os.RemoteException;
import android.os.UserHandle; import android.os.UserHandle;
import android.os.storage.StorageManagerInternal; import android.os.storage.StorageManagerInternal;
import android.os.storage.StorageVolume; import android.os.storage.StorageVolume;
import android.service.storage.ExternalStorageService; import android.service.storage.ExternalStorageService;
import android.service.storage.IExternalStorageService; import android.service.storage.IExternalStorageService;
import android.text.TextUtils;
import android.util.Slog; import android.util.Slog;
import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.Preconditions; import com.android.internal.util.Preconditions;
import com.android.server.LocalServices; import com.android.server.LocalServices;
import com.android.server.pm.UserManagerInternal;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.Map; import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
/** /**
* Controls the lifecycle of the {@link ActiveConnection} to an {@link ExternalStorageService} * Controls the lifecycle of the {@link ActiveConnection} to an {@link ExternalStorageService}
@@ -66,25 +66,20 @@ public final class StorageUserConnection {
private static final int DEFAULT_REMOTE_TIMEOUT_SECONDS = 20; private static final int DEFAULT_REMOTE_TIMEOUT_SECONDS = 20;
private final Object mLock = new Object(); private final Object mSessionsLock = new Object();
private final Context mContext; private final Context mContext;
private final int mUserId; private final int mUserId;
private final StorageSessionController mSessionController; private final StorageSessionController mSessionController;
private final ActiveConnection mActiveConnection = new ActiveConnection(); private final ActiveConnection mActiveConnection = new ActiveConnection();
private final boolean mIsDemoUser;
@GuardedBy("mLock") private final Map<String, Session> mSessions = new HashMap<>(); @GuardedBy("mLock") private final Map<String, Session> mSessions = new HashMap<>();
@GuardedBy("mLock") @Nullable private HandlerThread mHandlerThread; private final HandlerThread mHandlerThread;
public StorageUserConnection(Context context, int userId, StorageSessionController controller) { public StorageUserConnection(Context context, int userId, StorageSessionController controller) {
mContext = Objects.requireNonNull(context); mContext = Objects.requireNonNull(context);
mUserId = Preconditions.checkArgumentNonnegative(userId); mUserId = Preconditions.checkArgumentNonnegative(userId);
mSessionController = controller; mSessionController = controller;
mIsDemoUser = LocalServices.getService(UserManagerInternal.class) mHandlerThread = new HandlerThread("StorageUserConnectionThread-" + mUserId);
.getUserInfo(userId).isDemo(); mHandlerThread.start();
if (mIsDemoUser) {
mHandlerThread = new HandlerThread("StorageUserConnectionThread-" + mUserId);
mHandlerThread.start();
}
} }
/** /**
@@ -101,13 +96,12 @@ public final class StorageUserConnection {
Objects.requireNonNull(upperPath); Objects.requireNonNull(upperPath);
Objects.requireNonNull(lowerPath); Objects.requireNonNull(lowerPath);
prepareRemote(); Session session = new Session(sessionId, upperPath, lowerPath);
synchronized (mLock) { synchronized (mSessionsLock) {
Preconditions.checkArgument(!mSessions.containsKey(sessionId)); Preconditions.checkArgument(!mSessions.containsKey(sessionId));
Session session = new Session(sessionId, upperPath, lowerPath);
mSessions.put(sessionId, session); mSessions.put(sessionId, session);
mActiveConnection.startSessionLocked(session, pfd);
} }
mActiveConnection.startSession(session, pfd);
} }
/** /**
@@ -121,10 +115,13 @@ public final class StorageUserConnection {
Objects.requireNonNull(sessionId); Objects.requireNonNull(sessionId);
Objects.requireNonNull(vol); Objects.requireNonNull(vol);
prepareRemote(); synchronized (mSessionsLock) {
synchronized (mLock) { if (!mSessions.containsKey(sessionId)) {
mActiveConnection.notifyVolumeStateChangedLocked(sessionId, vol); Slog.i(TAG, "No session found for sessionId: " + sessionId);
return;
}
} }
mActiveConnection.notifyVolumeStateChanged(sessionId, vol);
} }
/** /**
@@ -135,7 +132,7 @@ public final class StorageUserConnection {
* with {@link #waitForExit}. * with {@link #waitForExit}.
**/ **/
public Session removeSession(String sessionId) { public Session removeSession(String sessionId) {
synchronized (mLock) { synchronized (mSessionsLock) {
return mSessions.remove(sessionId); return mSessions.remove(sessionId);
} }
} }
@@ -153,10 +150,7 @@ public final class StorageUserConnection {
} }
Slog.i(TAG, "Waiting for session end " + session + " ..."); Slog.i(TAG, "Waiting for session end " + session + " ...");
prepareRemote(); mActiveConnection.endSession(session);
synchronized (mLock) {
mActiveConnection.endSessionLocked(session);
}
} }
/** Restarts all available sessions for a user without blocking. /** Restarts all available sessions for a user without blocking.
@@ -164,7 +158,7 @@ public final class StorageUserConnection {
* Any failures will be ignored. * Any failures will be ignored.
**/ **/
public void resetUserSessions() { public void resetUserSessions() {
synchronized (mLock) { synchronized (mSessionsLock) {
if (mSessions.isEmpty()) { if (mSessions.isEmpty()) {
// Nothing to reset if we have no sessions to restart; we typically // Nothing to reset if we have no sessions to restart; we typically
// hit this path if the user was consciously shut down. // hit this path if the user was consciously shut down.
@@ -179,7 +173,7 @@ public final class StorageUserConnection {
* Removes all sessions, without waiting. * Removes all sessions, without waiting.
*/ */
public void removeAllSessions() { public void removeAllSessions() {
synchronized (mLock) { synchronized (mSessionsLock) {
Slog.i(TAG, "Removing " + mSessions.size() + " sessions for user: " + mUserId + "..."); Slog.i(TAG, "Removing " + mSessions.size() + " sessions for user: " + mUserId + "...");
mSessions.clear(); mSessions.clear();
} }
@@ -191,68 +185,54 @@ public final class StorageUserConnection {
*/ */
public void close() { public void close() {
mActiveConnection.close(); mActiveConnection.close();
if (mIsDemoUser) { mHandlerThread.quit();
mHandlerThread.quit();
}
} }
/** Returns all created sessions. */ /** Returns all created sessions. */
public Set<String> getAllSessionIds() { public Set<String> getAllSessionIds() {
synchronized (mLock) { synchronized (mSessionsLock) {
return new HashSet<>(mSessions.keySet()); return new HashSet<>(mSessions.keySet());
} }
} }
private void prepareRemote() throws ExternalStorageServiceException { @FunctionalInterface
try { interface AsyncStorageServiceCall {
waitForLatch(mActiveConnection.bind(), "remote_prepare_user " + mUserId); void run(@NonNull IExternalStorageService service, RemoteCallback callback) throws
} catch (IllegalStateException | TimeoutException e) { RemoteException;
throw new ExternalStorageServiceException("Failed to prepare remote", e);
}
}
private void waitForLatch(CountDownLatch latch, String reason) throws TimeoutException {
try {
if (!latch.await(DEFAULT_REMOTE_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
// TODO(b/140025078): Call ActivityManager ANR API?
Slog.wtf(TAG, "Failed to bind to the ExternalStorageService for user " + mUserId);
throw new TimeoutException("Latch wait for " + reason + " elapsed");
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new IllegalStateException("Latch wait for " + reason + " interrupted");
}
} }
private final class ActiveConnection implements AutoCloseable { private final class ActiveConnection implements AutoCloseable {
private final Object mLock = new Object();
// Lifecycle connection to the external storage service, needed to unbind. // Lifecycle connection to the external storage service, needed to unbind.
@GuardedBy("mLock") @Nullable private ServiceConnection mServiceConnection; @GuardedBy("mLock") @Nullable private ServiceConnection mServiceConnection;
// True if we are connecting, either bound or binding
// False && mRemote != null means we are connected // A future that holds the remote interface
// False && mRemote == null means we are neither connecting nor connected @GuardedBy("mLock")
@GuardedBy("mLock") @Nullable private boolean mIsConnecting; @Nullable private CompletableFuture<IExternalStorageService> mRemoteFuture;
// Binder object representing the external storage service.
// Non-null indicates we are connected // A list of outstanding futures for async calls, for which we are still waiting
@GuardedBy("mLock") @Nullable private IExternalStorageService mRemote; // for a callback. Used to unblock waiters if the service dies.
// Exception, if any, thrown from #startSessionLocked or #endSessionLocked @GuardedBy("mLock")
// Local variables cannot be referenced from a lambda expression :( so we private ArrayList<CompletableFuture<Void>> mOutstandingOps = new ArrayList<>();
// save the exception received in the callback here. Since we guard access
// (and clear the exception state) with the same lock which we hold during
// the entire transaction, there is no risk of race.
@GuardedBy("mLock") @Nullable private ParcelableException mLastException;
// Not guarded by any lock intentionally and non final because we cannot
// reset latches so need to create a new one after one use
private CountDownLatch mLatch;
@Override @Override
public void close() { public void close() {
ServiceConnection oldConnection = null; ServiceConnection oldConnection = null;
synchronized (mLock) { synchronized (mLock) {
Slog.i(TAG, "Closing connection for user " + mUserId); Slog.i(TAG, "Closing connection for user " + mUserId);
mIsConnecting = false;
oldConnection = mServiceConnection; oldConnection = mServiceConnection;
mServiceConnection = null; mServiceConnection = null;
mRemote = null; if (mRemoteFuture != null) {
// Let folks who are waiting for the connection know it ain't gonna happen
mRemoteFuture.cancel(true);
mRemoteFuture = null;
}
// Let folks waiting for callbacks from the remote know it ain't gonna happen
for (CompletableFuture<Void> op : mOutstandingOps) {
op.cancel(true);
}
mOutstandingOps.clear();
} }
if (oldConnection != null) { if (oldConnection != null) {
@@ -266,37 +246,37 @@ public final class StorageUserConnection {
} }
} }
public boolean isActiveLocked(Session session) { private void waitForAsync(AsyncStorageServiceCall asyncCall) throws Exception {
if (!session.isInitialisedLocked()) { CompletableFuture<IExternalStorageService> serviceFuture = connectIfNeeded();
Slog.i(TAG, "Session not initialised " + session); CompletableFuture<Void> opFuture = new CompletableFuture<>();
return false;
}
if (mRemote == null) { try {
throw new IllegalStateException("Valid session with inactive connection"); synchronized (mLock) {
mOutstandingOps.add(opFuture);
}
serviceFuture.thenCompose(service -> {
try {
asyncCall.run(service,
new RemoteCallback(result -> setResult(result, opFuture)));
} catch (RemoteException e) {
opFuture.completeExceptionally(e);
}
return opFuture;
}).get(DEFAULT_REMOTE_TIMEOUT_SECONDS, TimeUnit.SECONDS);
} finally {
synchronized (mLock) {
mOutstandingOps.remove(opFuture);
}
} }
return true;
} }
public void startSessionLocked(Session session, ParcelFileDescriptor fd) public void startSession(Session session, ParcelFileDescriptor fd)
throws ExternalStorageServiceException { throws ExternalStorageServiceException {
if (!isActiveLocked(session)) {
try {
fd.close();
} catch (IOException e) {
// ignore
}
return;
}
CountDownLatch latch = new CountDownLatch(1);
try { try {
mRemote.startSession(session.sessionId, waitForAsync((service, callback) -> service.startSession(session.sessionId,
FLAG_SESSION_TYPE_FUSE | FLAG_SESSION_ATTRIBUTE_INDEXABLE, FLAG_SESSION_TYPE_FUSE | FLAG_SESSION_ATTRIBUTE_INDEXABLE,
fd, session.upperPath, session.lowerPath, new RemoteCallback(result -> fd, session.upperPath, session.lowerPath, callback));
setResultLocked(latch, result)));
waitForLatch(latch, "start_session " + session);
maybeThrowExceptionLocked();
} catch (Exception e) { } catch (Exception e) {
throw new ExternalStorageServiceException("Failed to start session: " + session, e); throw new ExternalStorageServiceException("Failed to start session: " + session, e);
} finally { } finally {
@@ -308,73 +288,49 @@ public final class StorageUserConnection {
} }
} }
public void endSessionLocked(Session session) throws ExternalStorageServiceException { public void endSession(Session session) throws ExternalStorageServiceException {
if (!isActiveLocked(session)) {
// Nothing to end, not started yet
return;
}
CountDownLatch latch = new CountDownLatch(1);
try { try {
mRemote.endSession(session.sessionId, new RemoteCallback(result -> waitForAsync((service, callback) ->
setResultLocked(latch, result))); service.endSession(session.sessionId, callback));
waitForLatch(latch, "end_session " + session);
maybeThrowExceptionLocked();
} catch (Exception e) { } catch (Exception e) {
throw new ExternalStorageServiceException("Failed to end session: " + session, e); throw new ExternalStorageServiceException("Failed to end session: " + session, e);
} }
} }
public void notifyVolumeStateChangedLocked(String sessionId, StorageVolume vol) throws
public void notifyVolumeStateChanged(String sessionId, StorageVolume vol) throws
ExternalStorageServiceException { ExternalStorageServiceException {
CountDownLatch latch = new CountDownLatch(1);
try { try {
mRemote.notifyVolumeStateChanged(sessionId, vol, new RemoteCallback( waitForAsync((service, callback) ->
result -> setResultLocked(latch, result))); service.notifyVolumeStateChanged(sessionId, vol, callback));
waitForLatch(latch, "notify_volume_state_changed " + vol);
maybeThrowExceptionLocked();
} catch (Exception e) { } catch (Exception e) {
throw new ExternalStorageServiceException("Failed to notify volume state changed " throw new ExternalStorageServiceException("Failed to notify volume state changed "
+ "for vol : " + vol, e); + "for vol : " + vol, e);
} }
} }
private void setResultLocked(CountDownLatch latch, Bundle result) { private void setResult(Bundle result, CompletableFuture<Void> future) {
mLastException = result.getParcelable(EXTRA_ERROR); ParcelableException ex = result.getParcelable(EXTRA_ERROR);
latch.countDown(); if (ex != null) {
} future.completeExceptionally(ex);
} else {
private void maybeThrowExceptionLocked() throws IOException { future.complete(null);
if (mLastException != null) {
ParcelableException lastException = mLastException;
mLastException = null;
try {
lastException.maybeRethrow(IOException.class);
} catch (IOException e) {
throw e;
}
throw new RuntimeException(lastException);
} }
} }
public CountDownLatch bind() throws ExternalStorageServiceException { private CompletableFuture<IExternalStorageService> connectIfNeeded() throws
ExternalStorageServiceException {
ComponentName name = mSessionController.getExternalStorageServiceComponentName(); ComponentName name = mSessionController.getExternalStorageServiceComponentName();
if (name == null) { if (name == null) {
// Not ready to bind // Not ready to bind
throw new ExternalStorageServiceException( throw new ExternalStorageServiceException(
"Not ready to bind to the ExternalStorageService for user " + mUserId); "Not ready to bind to the ExternalStorageService for user " + mUserId);
} }
synchronized (mLock) { synchronized (mLock) {
if (mRemote != null || mIsConnecting) { if (mRemoteFuture != null) {
// Connected or connecting (bound or binding) return mRemoteFuture;
// Will wait on a latch that will countdown when we connect, unless we are }
// connected and the latch has already countdown, yay! CompletableFuture<IExternalStorageService> future = new CompletableFuture<>();
return mLatch;
} // else neither connected nor connecting
mLatch = new CountDownLatch(1);
mIsConnecting = true;
mServiceConnection = new ServiceConnection() { mServiceConnection = new ServiceConnection() {
@Override @Override
public void onServiceConnected(ComponentName name, IBinder service) { public void onServiceConnected(ComponentName name, IBinder service) {
@@ -406,16 +362,9 @@ public final class StorageUserConnection {
private void handleConnection(IBinder service) { private void handleConnection(IBinder service) {
synchronized (mLock) { synchronized (mLock) {
if (mIsConnecting) { future.complete(
mRemote = IExternalStorageService.Stub.asInterface(service); IExternalStorageService.Stub.asInterface(service));
mIsConnecting = false;
mLatch.countDown();
// Separate thread so we don't block the main thead
return;
}
} }
Slog.wtf(TAG, "Connection closed to the ExternalStorageService for user "
+ mUserId);
} }
private void handleDisconnection() { private void handleDisconnection() {
@@ -429,32 +378,19 @@ public final class StorageUserConnection {
}; };
Slog.i(TAG, "Binding to the ExternalStorageService for user " + mUserId); Slog.i(TAG, "Binding to the ExternalStorageService for user " + mUserId);
if (mIsDemoUser) { // Schedule on a worker thread, because the system server main thread can be
// Schedule on a worker thread for demo user to avoid deadlock // very busy early in boot.
if (mContext.bindServiceAsUser(new Intent().setComponent(name), if (mContext.bindServiceAsUser(new Intent().setComponent(name),
mServiceConnection, mServiceConnection,
Context.BIND_AUTO_CREATE | Context.BIND_IMPORTANT, Context.BIND_AUTO_CREATE | Context.BIND_IMPORTANT,
mHandlerThread.getThreadHandler(), mHandlerThread.getThreadHandler(),
UserHandle.of(mUserId))) { UserHandle.of(mUserId))) {
Slog.i(TAG, "Bound to the ExternalStorageService for user " + mUserId); Slog.i(TAG, "Bound to the ExternalStorageService for user " + mUserId);
return mLatch; mRemoteFuture = future;
} else { return future;
mIsConnecting = false;
throw new ExternalStorageServiceException(
"Failed to bind to the ExternalStorageService for user " + mUserId);
}
} else { } else {
if (mContext.bindServiceAsUser(new Intent().setComponent(name), throw new ExternalStorageServiceException(
mServiceConnection, "Failed to bind to the ExternalStorageService for user " + mUserId);
Context.BIND_AUTO_CREATE | Context.BIND_IMPORTANT,
UserHandle.of(mUserId))) {
Slog.i(TAG, "Bound to the ExternalStorageService for user " + mUserId);
return mLatch;
} else {
mIsConnecting = false;
throw new ExternalStorageServiceException(
"Failed to bind to the ExternalStorageService for user " + mUserId);
}
} }
} }
} }
@@ -476,10 +412,5 @@ public final class StorageUserConnection {
return "[SessionId: " + sessionId + ". UpperPath: " + upperPath + ". LowerPath: " return "[SessionId: " + sessionId + ". UpperPath: " + upperPath + ". LowerPath: "
+ lowerPath + "]"; + lowerPath + "]";
} }
@GuardedBy("mLock")
public boolean isInitialisedLocked() {
return !TextUtils.isEmpty(upperPath) && !TextUtils.isEmpty(lowerPath);
}
} }
} }