From bdf2a80816a1bcb58f09ee989a995983638aa894 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Mon, 8 May 2017 16:09:42 -0700 Subject: [PATCH] Always throw a ConnectionException on error There was a single case where we would return a 'null' for the service connect; if bindServiceAsUser() returned 'false'. Handle this condition with an exception. Also make the exception descriptive enough so we can add the correct resolution status to the resolution logs. Change-Id: I6cee25adfa74745579577cb9726f8dd9c2950746 Fixes: 38131147 Test: Manual; restart the resolver and see the exceptions --- .../pm/EphemeralResolverConnection.java | 57 ++++++++---- .../android/server/pm/InstantAppResolver.java | 89 ++++++++++++++----- 2 files changed, 107 insertions(+), 39 deletions(-) diff --git a/services/core/java/com/android/server/pm/EphemeralResolverConnection.java b/services/core/java/com/android/server/pm/EphemeralResolverConnection.java index 09e9433c9a022..aa780cc365237 100644 --- a/services/core/java/com/android/server/pm/EphemeralResolverConnection.java +++ b/services/core/java/com/android/server/pm/EphemeralResolverConnection.java @@ -80,19 +80,23 @@ final class EphemeralResolverConnection implements DeathRecipient { } public final List getInstantAppResolveInfoList(int hashPrefix[], - String token) { + String token) throws ConnectionException { throwIfCalledOnMainThread(); IInstantAppResolver target = null; try { - target = getRemoteInstanceLazy(token); - return mGetEphemeralResolveInfoCaller - .getEphemeralResolveInfoList(target, hashPrefix, token); - } catch (RemoteException e) { - } catch (InterruptedException | TimeoutException e) { - if (target == null) { - Slog.w(TAG, "[" + token + "] Timeout! Phase1 binding to instant app resolver"); - } else { - Slog.w(TAG, "[" + token + "] Timeout! Phase1 resolving instant app"); + try { + target = getRemoteInstanceLazy(token); + } catch (TimeoutException e) { + throw new ConnectionException(ConnectionException.FAILURE_BIND); + } catch (InterruptedException e) { + throw new ConnectionException(ConnectionException.FAILURE_INTERRUPTED); + } + try { + return mGetEphemeralResolveInfoCaller + .getEphemeralResolveInfoList(target, hashPrefix, token); + } catch (TimeoutException e) { + throw new ConnectionException(ConnectionException.FAILURE_BIND); + } catch (RemoteException ignore) { } } finally { synchronized (mLock) { @@ -104,7 +108,7 @@ final class EphemeralResolverConnection implements DeathRecipient { public final void getInstantAppIntentFilterList(int hashPrefix[], String token, String hostName, PhaseTwoCallback callback, Handler callbackHandler, - final long startTime) { + final long startTime) throws ConnectionException { final IRemoteCallback remoteCallback = new IRemoteCallback.Stub() { @Override public void sendResult(Bundle data) throws RemoteException { @@ -122,14 +126,16 @@ final class EphemeralResolverConnection implements DeathRecipient { try { getRemoteInstanceLazy(token) .getInstantAppIntentFilterList(hashPrefix, token, hostName, remoteCallback); - } catch (RemoteException e) { - } catch (InterruptedException | TimeoutException e) { - Slog.w(TAG, "[" + token + "] Timeout! Phase2 binding to instant app resolver"); + } catch (TimeoutException e) { + throw new ConnectionException(ConnectionException.FAILURE_BIND); + } catch (InterruptedException e) { + throw new ConnectionException(ConnectionException.FAILURE_INTERRUPTED); + } catch (RemoteException ignore) { } } private IInstantAppResolver getRemoteInstanceLazy(String token) - throws TimeoutException, InterruptedException { + throws ConnectionException, TimeoutException, InterruptedException { synchronized (mLock) { if (mRemoteInstance != null) { return mRemoteInstance; @@ -139,7 +145,7 @@ final class EphemeralResolverConnection implements DeathRecipient { } } - private void waitForBind(String token) throws TimeoutException, InterruptedException { + private void waitForBindLocked(String token) throws TimeoutException, InterruptedException { final long startMillis = SystemClock.uptimeMillis(); while (mIsBinding) { if (mRemoteInstance != null) { @@ -154,12 +160,13 @@ final class EphemeralResolverConnection implements DeathRecipient { } } - private void bindLocked(String token) throws TimeoutException, InterruptedException { + private void bindLocked(String token) + throws ConnectionException, TimeoutException, InterruptedException { if (DEBUG_EPHEMERAL && mIsBinding && mRemoteInstance == null) { Slog.i(TAG, "[" + token + "] Previous bind timed out; waiting for connection"); } try { - waitForBind(token); + waitForBindLocked(token); } catch (TimeoutException e) { if (DEBUG_EPHEMERAL) { Slog.i(TAG, "[" + token + "] Previous connection never established; rebinding"); @@ -179,9 +186,10 @@ final class EphemeralResolverConnection implements DeathRecipient { wasBound = mContext .bindServiceAsUser(mIntent, mServiceConnection, flags, UserHandle.SYSTEM); if (wasBound) { - waitForBind(token); + waitForBindLocked(token); } else { Slog.w(TAG, "[" + token + "] Failed to bind to: " + mIntent); + throw new ConnectionException(ConnectionException.FAILURE_BIND); } } finally { mIsBinding = wasBound && mRemoteInstance == null; @@ -222,6 +230,17 @@ final class EphemeralResolverConnection implements DeathRecipient { List instantAppResolveInfoList, long startTime); } + public static class ConnectionException extends Exception { + public static final int FAILURE_BIND = 1; + public static final int FAILURE_CALL = 2; + public static final int FAILURE_INTERRUPTED = 3; + + public final int failure; + public ConnectionException(int _failure) { + failure = _failure; + } + } + private final class MyServiceConnection implements ServiceConnection { @Override public void onServiceConnected(ComponentName name, IBinder service) { diff --git a/services/core/java/com/android/server/pm/InstantAppResolver.java b/services/core/java/com/android/server/pm/InstantAppResolver.java index f3c9cbbdbf6cf..85be4a2c64820 100644 --- a/services/core/java/com/android/server/pm/InstantAppResolver.java +++ b/services/core/java/com/android/server/pm/InstantAppResolver.java @@ -22,6 +22,7 @@ import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.FIELD_ import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.FIELD_INSTANT_APP_RESOLUTION_DELAY_MS; import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.FIELD_INSTANT_APP_RESOLUTION_STATUS; +import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.app.ActivityManager; @@ -50,20 +51,37 @@ import android.util.Slog; import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto; +import com.android.server.pm.EphemeralResolverConnection.ConnectionException; import com.android.server.pm.EphemeralResolverConnection.PhaseTwoCallback; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.UUID; +import java.util.concurrent.TimeoutException; /** @hide */ public abstract class InstantAppResolver { private static final boolean DEBUG_EPHEMERAL = Build.IS_DEBUGGABLE; private static final String TAG = "PackageManager"; - private static int RESOLUTION_SUCCESS = 0; - private static int RESOLUTION_FAILURE = 1; + private static final int RESOLUTION_SUCCESS = 0; + private static final int RESOLUTION_FAILURE = 1; + /** Binding to the external service timed out */ + private static final int RESOLUTION_BIND_TIMEOUT = 2; + /** The call to retrieve an instant application response timed out */ + private static final int RESOLUTION_CALL_TIMEOUT = 3; + + @IntDef(flag = true, prefix = { "RESOLUTION_" }, value = { + RESOLUTION_SUCCESS, + RESOLUTION_FAILURE, + RESOLUTION_BIND_TIMEOUT, + RESOLUTION_CALL_TIMEOUT, + }) + @Retention(RetentionPolicy.SOURCE) + public @interface ResolutionStatus {} private static MetricsLogger sMetricsLogger; private static MetricsLogger getLogger() { @@ -78,29 +96,43 @@ public abstract class InstantAppResolver { final long startTime = System.currentTimeMillis(); final String token = UUID.randomUUID().toString(); if (DEBUG_EPHEMERAL) { - Log.d(TAG, "[" + token + "] Resolving phase 1"); + Log.d(TAG, "[" + token + "] Phase1; resolving"); } final Intent intent = requestObj.origIntent; final InstantAppDigest digest = new InstantAppDigest(intent.getData().getHost(), 5 /*maxDigests*/); final int[] shaPrefix = digest.getDigestPrefix(); - final List instantAppResolveInfoList = - connection.getInstantAppResolveInfoList(shaPrefix, token); - - if (instantAppResolveInfoList == null || instantAppResolveInfoList.size() == 0) { - // No hash prefix match; there are no instant apps for this domain. - if (DEBUG_EPHEMERAL) { - Log.d(TAG, "[" + token + "] No results returned"); + AuxiliaryResolveInfo resolveInfo = null; + @ResolutionStatus int resolutionStatus = RESOLUTION_SUCCESS; + try { + final List instantAppResolveInfoList = + connection.getInstantAppResolveInfoList(shaPrefix, token); + if (instantAppResolveInfoList != null && instantAppResolveInfoList.size() > 0) { + resolveInfo = InstantAppResolver.filterInstantAppIntent( + instantAppResolveInfoList, intent, requestObj.resolvedType, + requestObj.userId, intent.getPackage(), digest, token); + } + } catch (ConnectionException e) { + if (e.failure == ConnectionException.FAILURE_BIND) { + resolutionStatus = RESOLUTION_BIND_TIMEOUT; + } else if (e.failure == ConnectionException.FAILURE_CALL) { + resolutionStatus = RESOLUTION_CALL_TIMEOUT; + } else { + resolutionStatus = RESOLUTION_FAILURE; } - return null; } - final AuxiliaryResolveInfo resolveInfo = InstantAppResolver.filterInstantAppIntent( - instantAppResolveInfoList, intent, requestObj.resolvedType, requestObj.userId, - intent.getPackage(), digest, token); logMetrics(ACTION_INSTANT_APP_RESOLUTION_PHASE_ONE, startTime, token, - RESOLUTION_SUCCESS); + resolutionStatus); if (DEBUG_EPHEMERAL && resolveInfo == null) { - Log.d(TAG, "[" + token + "] No results matched"); + if (resolutionStatus == RESOLUTION_BIND_TIMEOUT) { + Log.d(TAG, "[" + token + "] Phase1; bind timed out"); + } else if (resolutionStatus == RESOLUTION_CALL_TIMEOUT) { + Log.d(TAG, "[" + token + "] Phase1; call timed out"); + } else if (resolutionStatus != RESOLUTION_SUCCESS) { + Log.d(TAG, "[" + token + "] Phase1; service connection error"); + } else { + Log.d(TAG, "[" + token + "] Phase1; No results matched"); + } } return resolveInfo; } @@ -111,7 +143,7 @@ public abstract class InstantAppResolver { final long startTime = System.currentTimeMillis(); final String token = requestObj.responseObj.token; if (DEBUG_EPHEMERAL) { - Log.d(TAG, "[" + token + "] Resolving phase 2"); + Log.d(TAG, "[" + token + "] Phase2; resolving"); } final Intent intent = requestObj.origIntent; final String hostName = intent.getData().getHost(); @@ -170,8 +202,24 @@ public abstract class InstantAppResolver { context.startActivity(installerIntent); } }; - connection.getInstantAppIntentFilterList( - shaPrefix, token, hostName, callback, callbackHandler, startTime); + try { + connection.getInstantAppIntentFilterList( + shaPrefix, token, hostName, callback, callbackHandler, startTime); + } catch (ConnectionException e) { + @ResolutionStatus int resolutionStatus = RESOLUTION_FAILURE; + if (e.failure == ConnectionException.FAILURE_BIND) { + resolutionStatus = RESOLUTION_BIND_TIMEOUT; + } + logMetrics(ACTION_INSTANT_APP_RESOLUTION_PHASE_TWO, startTime, token, + resolutionStatus); + if (DEBUG_EPHEMERAL) { + if (resolutionStatus == RESOLUTION_BIND_TIMEOUT) { + Log.d(TAG, "[" + token + "] Phase2; bind timed out"); + } else { + Log.d(TAG, "[" + token + "] Phase2; service connection error"); + } + } + } } /** @@ -322,7 +370,8 @@ public abstract class InstantAppResolver { return null; } - private static void logMetrics(int action, long startTime, String token, int status) { + private static void logMetrics(int action, long startTime, String token, + @ResolutionStatus int status) { final LogMaker logMaker = new LogMaker(action) .setType(MetricsProto.MetricsEvent.TYPE_ACTION) .addTaggedData(FIELD_INSTANT_APP_RESOLUTION_DELAY_MS,