From c96c83ea68655ded78b31adc51c72092811093dd Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Mon, 14 Nov 2022 16:03:12 -0800 Subject: [PATCH] [pm/metrics][6/n] log install failures snapshot is no longer needed because the installerPackageUid is now part of InstallSource, which we can log for both succeeded and failed installs. Notice that we still need to track verification errors. BUG: 249294752 Test: manual Change-Id: I9a39888d7bb1b200ac9e77c596b0560cf96a36b5 --- .../com/android/server/pm/InstallRequest.java | 17 ++++- .../android/server/pm/InstallingSession.java | 62 +++++++++++++------ .../com/android/server/pm/PackageMetrics.java | 43 ++++++++----- 3 files changed, 83 insertions(+), 39 deletions(-) diff --git a/services/core/java/com/android/server/pm/InstallRequest.java b/services/core/java/com/android/server/pm/InstallRequest.java index 6271eaca30c26..71571dc4f3066 100644 --- a/services/core/java/com/android/server/pm/InstallRequest.java +++ b/services/core/java/com/android/server/pm/InstallRequest.java @@ -81,7 +81,7 @@ final class InstallRequest { /** Package Installed Info */ @Nullable private String mName; - private int mUid = -1; + private int mUid = INVALID_UID; // The set of users that originally had this package installed. @Nullable private int[] mOrigUsers; @@ -317,6 +317,11 @@ final class InstallRequest { ? mInstallArgs.mInstallSource.mInstallerPackageName : null; } + public int getInstallerPackageUid() { + return (mInstallArgs != null && mInstallArgs.mInstallSource != null) + ? mInstallArgs.mInstallSource.mInstallerPackageUid : INVALID_UID; + } + public int getDataLoaderType() { return mInstallArgs == null ? DataLoaderType.NONE : mInstallArgs.mDataLoaderType; } @@ -608,12 +613,18 @@ final class InstallRequest { setReturnCode(code); setReturnMessage(msg); Slog.w(TAG, msg); + if (mPackageMetrics != null) { + mPackageMetrics.onInstallFailed(); + } } public void setError(String msg, PackageManagerException e) { mReturnCode = e.error; setReturnMessage(ExceptionUtils.getCompleteMessage(msg, e)); Slog.w(TAG, msg, e); + if (mPackageMetrics != null) { + mPackageMetrics.onInstallFailed(); + } } public void setReturnCode(int returnCode) { @@ -757,10 +768,10 @@ final class InstallRequest { } } - public void onInstallCompleted(Computer snapshot) { + public void onInstallCompleted() { if (getReturnCode() == INSTALL_SUCCEEDED) { if (mPackageMetrics != null) { - mPackageMetrics.onInstallSucceed(snapshot); + mPackageMetrics.onInstallSucceed(); } } } diff --git a/services/core/java/com/android/server/pm/InstallingSession.java b/services/core/java/com/android/server/pm/InstallingSession.java index 6b3490399f89e..69ced1b39585d 100644 --- a/services/core/java/com/android/server/pm/InstallingSession.java +++ b/services/core/java/com/android/server/pm/InstallingSession.java @@ -223,7 +223,7 @@ class InstallingSession { * policy if needed and then create install arguments based * on the install location. */ - private void handleStartCopy() { + private void handleStartCopy(InstallRequest request) { if ((mInstallFlags & PackageManager.INSTALL_APEX) != 0) { mRet = INSTALL_SUCCEEDED; return; @@ -239,6 +239,7 @@ class InstallingSession { pkgLite, mRequiredInstalledVersionCode, mInstallFlags); mRet = ret.first; if (mRet != INSTALL_SUCCEEDED) { + request.setError(mRet, "Failed to verify version code"); return; } } @@ -258,14 +259,16 @@ class InstallingSession { } mRet = overrideInstallLocation(pkgLite.packageName, pkgLite.recommendedInstallLocation, pkgLite.installLocation); + if (mRet != INSTALL_SUCCEEDED) { + request.setError(mRet, "Failed to override installation location"); + } } - private void handleReturnCode() { - processPendingInstall(); + private void handleReturnCode(InstallRequest installRequest) { + processPendingInstall(installRequest); } - private void processPendingInstall() { - InstallRequest installRequest = new InstallRequest(this); + private void processPendingInstall(InstallRequest installRequest) { if (mRet == PackageManager.INSTALL_SUCCEEDED) { mRet = copyApk(installRequest); } @@ -302,21 +305,26 @@ class InstallingSession { request.setCodeFile(mOriginInfo.mFile); return PackageManager.INSTALL_SUCCEEDED; } - + int ret; try { final boolean isEphemeral = (mInstallFlags & PackageManager.INSTALL_INSTANT_APP) != 0; request.setCodeFile( mPm.mInstallerService.allocateStageDirLegacy(mVolumeUuid, isEphemeral)); } catch (IOException e) { - Slog.w(TAG, "Failed to create copy file: " + e); - return PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; + final String errorMessage = "Failed to create copy file"; + Slog.w(TAG, errorMessage + ": " + e); + ret = PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; + request.setError(ret, errorMessage); + return ret; } - int ret = PackageManagerServiceUtils.copyPackage( + ret = PackageManagerServiceUtils.copyPackage( mOriginInfo.mFile.getAbsolutePath(), request.getCodeFile()); if (ret != PackageManager.INSTALL_SUCCEEDED) { - Slog.e(TAG, "Failed to copy package"); + final String errorMessage = "Failed to copy package"; + Slog.e(TAG, errorMessage); + request.setError(ret, errorMessage); return ret; } @@ -329,8 +337,10 @@ class InstallingSession { ret = NativeLibraryHelper.copyNativeBinariesWithOverride(handle, libraryRoot, request.getAbiOverride(), isIncremental); } catch (IOException e) { - Slog.e(TAG, "Copying native libraries failed", e); + final String errorMessage = "Copying native libraries failed"; + Slog.e(TAG, errorMessage, e); ret = PackageManager.INSTALL_FAILED_INTERNAL_ERROR; + request.setError(ret, errorMessage); } finally { IoUtils.closeQuietly(handle); } @@ -352,8 +362,11 @@ class InstallingSession { mMoveInfo.mPackageName, mMoveInfo.mAppId, mMoveInfo.mSeInfo, mMoveInfo.mTargetSdkVersion, mMoveInfo.mFromCodePath); } catch (Installer.InstallerException e) { - Slog.w(TAG, "Failed to move app", e); - return PackageManager.INSTALL_FAILED_INTERNAL_ERROR; + final String errorMessage = "Failed to move app"; + final int ret = PackageManager.INSTALL_FAILED_INTERNAL_ERROR; + request.setError(ret, errorMessage); + Slog.w(TAG, errorMessage, e); + return ret; } } @@ -456,8 +469,9 @@ class InstallingSession { Trace.asyncTraceEnd(TRACE_TAG_PACKAGE_MANAGER, "queueInstall", System.identityHashCode(this)); Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "startInstall"); - handleStartCopy(); - handleReturnCode(); + InstallRequest installRequest = new InstallRequest(this); + handleStartCopy(installRequest); + handleReturnCode(installRequest); Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); } @@ -518,7 +532,7 @@ class InstallingSession { mInstallPackageHelper.installPackagesTraced(installRequests); for (InstallRequest request : installRequests) { - request.onInstallCompleted(mPm.snapshotComputer()); + request.onInstallCompleted(); doPostInstall(request); } } @@ -635,11 +649,18 @@ class InstallingSession { Trace.asyncTraceEnd(TRACE_TAG_PACKAGE_MANAGER, "queueInstall", System.identityHashCode(this)); Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "start"); - for (InstallingSession childInstallingSession : mChildInstallingSessions) { - childInstallingSession.handleStartCopy(); + + final int numChildSessions = mChildInstallingSessions.size(); + final ArrayList installRequests = new ArrayList<>(numChildSessions); + + for (int i = 0; i < numChildSessions; i++) { + final InstallingSession childSession = mChildInstallingSessions.get(i); + final InstallRequest installRequest = new InstallRequest(childSession); + installRequests.add(installRequest); + childSession.handleStartCopy(installRequest); } - for (InstallingSession childInstallingSession : mChildInstallingSessions) { - childInstallingSession.handleReturnCode(); + for (int i = 0; i < numChildSessions; i++) { + mChildInstallingSessions.get(i).handleReturnCode(installRequests.get(i)); } Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); } @@ -647,6 +668,7 @@ class InstallingSession { public void tryProcessInstallRequest(InstallRequest request) { mCurrentInstallRequests.add(request); if (mCurrentInstallRequests.size() != mChildInstallingSessions.size()) { + // Wait until all the installRequests have finished copying return; } int completeStatus = PackageManager.INSTALL_SUCCEEDED; diff --git a/services/core/java/com/android/server/pm/PackageMetrics.java b/services/core/java/com/android/server/pm/PackageMetrics.java index 0574f737a54b6..3dcf9260f8297 100644 --- a/services/core/java/com/android/server/pm/PackageMetrics.java +++ b/services/core/java/com/android/server/pm/PackageMetrics.java @@ -16,8 +16,6 @@ package com.android.server.pm; -import static android.os.Process.INVALID_UID; - import android.annotation.IntDef; import android.content.pm.PackageManager; import android.content.pm.parsing.ApkLiteParseUtils; @@ -26,7 +24,6 @@ import android.util.SparseArray; import com.android.internal.util.FrameworkStatsLog; import com.android.server.LocalServices; -import com.android.server.pm.pkg.PackageStateInternal; import java.io.File; import java.io.IOException; @@ -68,32 +65,46 @@ final class PackageMetrics { mInstallRequest = installRequest; } - public void onInstallSucceed(Computer snapshot) { + public void onInstallSucceed() { // TODO(b/239722919): report to SecurityLog if on work profile or managed device - reportInstallationStats(snapshot, true /* success */); + reportInstallationStats(true /* success */); } - private void reportInstallationStats(Computer snapshot, boolean success) { + public void onInstallFailed() { + reportInstallationStats(false /* success */); + } + + private void reportInstallationStats(boolean success) { UserManagerInternal userManagerInternal = LocalServices.getService(UserManagerInternal.class); - // TODO(b/249294752): do not log if adb final long installDurationMillis = System.currentTimeMillis() - mInstallStartTimestampMillis; // write to stats final Pair stepDurations = getInstallStepDurations(); final int[] newUsers = mInstallRequest.getNewUsers(); final int[] originalUsers = mInstallRequest.getOriginUsers(); - final String packageName = mInstallRequest.getName(); - final String installerPackageName = mInstallRequest.getInstallerPackageName(); - final int installerUid = installerPackageName == null ? INVALID_UID - : snapshot.getPackageUid(installerPackageName, 0, 0); - final PackageStateInternal ps = snapshot.getPackageStateInternal(packageName); - final long versionCode = success ? 0 : ps.getVersionCode(); - final long apksSize = getApksSize(ps.getPath()); + final String packageName; + // only reporting package name for failed non-adb installations + if (success || mInstallRequest.isInstallFromAdb()) { + packageName = null; + } else { + packageName = mInstallRequest.getName(); + } + + final int installerPackageUid = mInstallRequest.getInstallerPackageUid(); + + long versionCode = 0, apksSize = 0; + if (success) { + final PackageSetting ps = mInstallRequest.getScannedPackageSetting(); + if (ps != null) { + versionCode = ps.getVersionCode(); + apksSize = getApksSize(ps.getPath()); + } + } FrameworkStatsLog.write(FrameworkStatsLog.PACKAGE_INSTALLATION_SESSION_REPORTED, mInstallRequest.getSessionId() /* session_id */, - success ? null : packageName /* not report package_name on success */, + packageName /* package_name */, mInstallRequest.getUid() /* uid */, newUsers /* user_ids */, userManagerInternal.getUserTypesForStatsd(newUsers) /* user_types */, @@ -107,7 +118,7 @@ final class PackageMetrics { stepDurations.second /* step_duration_millis */, installDurationMillis /* total_duration_millis */, mInstallRequest.getInstallFlags() /* install_flags */, - installerUid /* installer_package_uid */, + installerPackageUid /* installer_package_uid */, -1 /* original_installer_package_uid */, mInstallRequest.getDataLoaderType() /* data_loader_type */, mInstallRequest.getRequireUserAction() /* user_action_required_type */,