From cdccfc72b5c0795d1de2b49dda3e88c7b49c2d8a Mon Sep 17 00:00:00 2001 From: Alex Buynytskyy Date: Mon, 17 May 2021 11:09:58 -0700 Subject: [PATCH] Only lookup dataLoader when necessary. This avoid the race condition when we are about to seal the session, but another notification goes through and fails the commit. Also saves some resources - we almost never need a dataLoader for e.g. Incremental installations. Bug: 188079906 Bug: 187967058 Fixes: 188079906 Fixes: 187967058 Test: atest PackageManagerShellCommandIncrementalTest Change-Id: Id931b40362f603ccd45b64ff5f8636d8c32e849e --- .../server/pm/PackageInstallerSession.java | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index a799ce20f5c7c..4b902ff62ad42 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -3754,13 +3754,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - final DataLoaderManager dataLoaderManager = mContext.getSystemService( - DataLoaderManager.class); - if (dataLoaderManager == null) { - throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, - "Failed to find data loader manager service"); - } - final DataLoaderParams params = this.params.dataLoaderParams; final boolean manualStartAndDestroy = !isIncrementalInstallation(); final boolean systemDataLoader = isSystemDataLoaderInstallation(); @@ -3785,20 +3778,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return; } try { - IDataLoader dataLoader = dataLoaderManager.getDataLoader(dataLoaderId); - if (dataLoader == null) { - mDataLoaderFinished = true; - dispatchSessionValidationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, - "Failure to obtain data loader"); - return; - } - switch (status) { case IDataLoaderStatusListener.DATA_LOADER_BOUND: { if (manualStartAndDestroy) { FileSystemControlParcel control = new FileSystemControlParcel(); control.callback = new FileSystemConnector(addedFiles); - dataLoader.create(dataLoaderId, params.getData(), control, this); + getDataLoader(dataLoaderId).create(dataLoaderId, params.getData(), + control, this); } break; @@ -3807,12 +3793,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (manualStartAndDestroy) { // IncrementalFileStorages will call start after all files are // created in IncFS. - dataLoader.start(dataLoaderId); + getDataLoader(dataLoaderId).start(dataLoaderId); } break; } case IDataLoaderStatusListener.DATA_LOADER_STARTED: { - dataLoader.prepareImage( + getDataLoader(dataLoaderId).prepareImage( dataLoaderId, addedFiles.toArray( new InstallationFileParcel[addedFiles.size()]), @@ -3828,7 +3814,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { dispatchSessionSealed(); } if (manualStartAndDestroy) { - dataLoader.destroy(dataLoaderId); + getDataLoader(dataLoaderId).destroy(dataLoaderId); } break; } @@ -3837,7 +3823,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { dispatchSessionValidationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, "Failed to prepare image."); if (manualStartAndDestroy) { - dataLoader.destroy(dataLoaderId); + getDataLoader(dataLoaderId).destroy(dataLoaderId); } break; } @@ -3852,11 +3838,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { break; } case IDataLoaderStatusListener.DATA_LOADER_UNRECOVERABLE: - mDataLoaderFinished = true; - dispatchSessionValidationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, + throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, "DataLoader reported unrecoverable failure."); - break; } + } catch (PackageManagerException e) { + mDataLoaderFinished = true; + dispatchSessionValidationFailure(e.error, ExceptionUtils.getCompleteMessage(e)); } catch (RemoteException e) { // In case of streaming failure we don't want to fail or commit the session. // Just return from this method and allow caller to commit again. @@ -3931,7 +3918,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } final long bindDelayMs = 0; - if (!dataLoaderManager.bindToDataLoader(sessionId, params.getData(), bindDelayMs, + if (!getDataLoaderManager().bindToDataLoader(sessionId, params.getData(), bindDelayMs, statusListener)) { throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, "Failed to initialize data loader"); @@ -3940,6 +3927,24 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return false; } + private DataLoaderManager getDataLoaderManager() throws PackageManagerException { + DataLoaderManager dataLoaderManager = mContext.getSystemService(DataLoaderManager.class); + if (dataLoaderManager == null) { + throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, + "Failed to find data loader manager service"); + } + return dataLoaderManager; + } + + private IDataLoader getDataLoader(int dataLoaderId) throws PackageManagerException { + IDataLoader dataLoader = getDataLoaderManager().getDataLoader(dataLoaderId); + if (dataLoader == null) { + throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, + "Failure to obtain data loader"); + } + return dataLoader; + } + private void dispatchSessionValidationFailure(int error, String detailMessage) { mHandler.obtainMessage(MSG_SESSION_VALIDATION_FAILURE, error, -1, detailMessage).sendToTarget();