Removing race condition accessing shared binder object.
Test: incrementally installing two apks at the same time
Bug: b/150411019
Change-Id: I81231edf7a32470542ec529aa305b4f9fb2b80e3
diff --git a/core/java/android/content/pm/IDataLoader.aidl b/core/java/android/content/pm/IDataLoader.aidl
index 6a2658d..5ec6341 100644
--- a/core/java/android/content/pm/IDataLoader.aidl
+++ b/core/java/android/content/pm/IDataLoader.aidl
@@ -29,9 +29,9 @@
void create(int id, in DataLoaderParamsParcel params,
in FileSystemControlParcel control,
IDataLoaderStatusListener listener);
- void start();
- void stop();
- void destroy();
+ void start(int id);
+ void stop(int id);
+ void destroy(int id);
- void prepareImage(in InstallationFileParcel[] addedFiles, in @utf8InCpp String[] removedFiles);
+ void prepareImage(int id, in InstallationFileParcel[] addedFiles, in @utf8InCpp String[] removedFiles);
}
diff --git a/core/java/android/service/dataloader/DataLoaderService.java b/core/java/android/service/dataloader/DataLoaderService.java
index d4db79e..0170726 100644
--- a/core/java/android/service/dataloader/DataLoaderService.java
+++ b/core/java/android/service/dataloader/DataLoaderService.java
@@ -102,21 +102,18 @@
}
private class DataLoaderBinderService extends IDataLoader.Stub {
- private int mId;
-
@Override
public void create(int id, @NonNull DataLoaderParamsParcel params,
@NonNull FileSystemControlParcel control,
@NonNull IDataLoaderStatusListener listener)
throws RuntimeException {
- mId = id;
try {
if (!nativeCreateDataLoader(id, control, params, listener)) {
- Slog.e(TAG, "Failed to create native loader for " + mId);
+ Slog.e(TAG, "Failed to create native loader for " + id);
}
} catch (Exception ex) {
- Slog.e(TAG, "Failed to create native loader for " + mId, ex);
- destroy();
+ Slog.e(TAG, "Failed to create native loader for " + id, ex);
+ destroy(id);
throw new RuntimeException(ex);
} finally {
// Closing FDs.
@@ -150,30 +147,31 @@
}
@Override
- public void start() {
- if (!nativeStartDataLoader(mId)) {
- Slog.e(TAG, "Failed to start loader: " + mId);
+ public void start(int id) {
+ if (!nativeStartDataLoader(id)) {
+ Slog.e(TAG, "Failed to start loader: " + id);
}
}
@Override
- public void stop() {
- if (!nativeStopDataLoader(mId)) {
- Slog.w(TAG, "Failed to stop loader: " + mId);
+ public void stop(int id) {
+ if (!nativeStopDataLoader(id)) {
+ Slog.w(TAG, "Failed to stop loader: " + id);
}
}
@Override
- public void destroy() {
- if (!nativeDestroyDataLoader(mId)) {
- Slog.w(TAG, "Failed to destroy loader: " + mId);
+ public void destroy(int id) {
+ if (!nativeDestroyDataLoader(id)) {
+ Slog.w(TAG, "Failed to destroy loader: " + id);
}
}
@Override
- public void prepareImage(InstallationFileParcel[] addedFiles, String[] removedFiles) {
- if (!nativePrepareImage(mId, addedFiles, removedFiles)) {
- Slog.w(TAG, "Failed to prepare image for data loader: " + mId);
+ public void prepareImage(int id, InstallationFileParcel[] addedFiles,
+ String[] removedFiles) {
+ if (!nativePrepareImage(id, addedFiles, removedFiles)) {
+ Slog.w(TAG, "Failed to prepare image for data loader: " + id);
}
}
}
diff --git a/services/core/java/com/android/server/pm/DataLoaderManagerService.java b/services/core/java/com/android/server/pm/DataLoaderManagerService.java
index 8eb773a..09baf6e 100644
--- a/services/core/java/com/android/server/pm/DataLoaderManagerService.java
+++ b/services/core/java/com/android/server/pm/DataLoaderManagerService.java
@@ -213,7 +213,7 @@
void destroy() {
try {
- mDataLoader.destroy();
+ mDataLoader.destroy(mId);
} catch (RemoteException ignored) {
}
mContext.unbindService(this);
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 28d7c13..308bc41 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -2582,12 +2582,13 @@
if (manualStartAndDestroy) {
// IncrementalFileStorages will call start after all files are
// created in IncFS.
- dataLoader.start();
+ dataLoader.start(dataLoaderId);
}
break;
}
case IDataLoaderStatusListener.DATA_LOADER_STARTED: {
dataLoader.prepareImage(
+ dataLoaderId,
addedFiles.toArray(
new InstallationFileParcel[addedFiles.size()]),
removedFiles.toArray(new String[removedFiles.size()]));
@@ -2602,7 +2603,7 @@
dispatchStreamValidateAndCommit();
}
if (manualStartAndDestroy) {
- dataLoader.destroy();
+ dataLoader.destroy(dataLoaderId);
}
break;
}
@@ -2612,7 +2613,7 @@
new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE,
"Failed to prepare image."));
if (manualStartAndDestroy) {
- dataLoader.destroy();
+ dataLoader.destroy(dataLoaderId);
}
break;
}
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 7275936..e452415 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -941,7 +941,7 @@
if (!dataloader) {
return false;
}
- status = dataloader->start();
+ status = dataloader->start(mountId);
if (!status.isOk()) {
return false;
}
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 6002226..f5b88d9 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -100,10 +100,11 @@
const sp<IDataLoaderStatusListener>&) override {
return binder::Status::ok();
}
- binder::Status start() override { return binder::Status::ok(); }
- binder::Status stop() override { return binder::Status::ok(); }
- binder::Status destroy() override { return binder::Status::ok(); }
- binder::Status prepareImage(const std::vector<InstallationFileParcel>&,
+ binder::Status start(int32_t) override { return binder::Status::ok(); }
+ binder::Status stop(int32_t) override { return binder::Status::ok(); }
+ binder::Status destroy(int32_t) override { return binder::Status::ok(); }
+ binder::Status prepareImage(int32_t,
+ const std::vector<InstallationFileParcel>&,
const std::vector<std::string>&) override {
return binder::Status::ok();
}