ART Services: Distinguish fatal and non-fatal errors.
Fatal errors should never happen. System server should crash if fatal
errors happen.
Non-fatal errors are error that service-art should handle (e.g., I/O
errors, sub-process crashes). The scope of the error depends on the
function that throws it, so service-art should catch the error at every
call site and take different actions.
Ideally, this should be a checked exception or an additional return
value that forces service-art to handle it, but
`ServiceSpecificException` (a separate runtime exception type) is the
best approximate we have given the limitation of Java and Binder.
This CL distinguishes them by defining fatal errors as
IllegalStateException and non-fatal errors as ServiceSpecificException.
This CL also adds helper functions and macros for throwing those errors
from artd.
Bug: 229268202
Test: atest ArtServiceTests
Ignore-AOSP-First: ART Services.
Change-Id: I9106d28a5af8e78988bdd1db427f191a65c09387
diff --git a/artd/artd.cc b/artd/artd.cc
index 87d3858..fc99b2a 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -53,6 +53,7 @@
using ::android::base::Result;
using ::android::base::Split;
using ::android::base::StringPrintf;
+using ::android::base::StringReplace;
using ::ndk::ScopedAStatus;
constexpr const char* kServiceName = "artd";
@@ -79,24 +80,55 @@
return size;
}
+std::string EscapeErrorMessage(const std::string& message) {
+ return StringReplace(message, std::string("\0", /*n=*/1), "\\0", /*all=*/true);
+}
+
+// Indicates an error that should never happen (e.g., illegal arguments passed by service-art
+// internally). System server should crash if this kind of error happens.
+ScopedAStatus Fatal(const std::string& message) {
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_STATE,
+ EscapeErrorMessage(message).c_str());
+}
+
+// Indicates an error that service-art should handle (e.g., I/O errors, sub-process crashes).
+// The scope of the error depends on the function that throws it, so service-art should catch the
+// error at every call site and take different actions.
+// Ideally, this should be a checked exception or an additional return value that forces service-art
+// to handle it, but `ServiceSpecificException` (a separate runtime exception type) is the best
+// approximate we have given the limitation of Java and Binder.
+ScopedAStatus NonFatal(const std::string& message) {
+ constexpr int32_t kArtdNonFatalErrorCode = 1;
+ return ScopedAStatus::fromServiceSpecificErrorWithMessage(kArtdNonFatalErrorCode,
+ EscapeErrorMessage(message).c_str());
+}
+
} // namespace
+#define OR_RETURN_ERROR(func, expr) \
+ ({ \
+ decltype(expr)&& tmp = (expr); \
+ if (!tmp.ok()) { \
+ return (func)(tmp.error().message()); \
+ } \
+ std::move(tmp).value(); \
+ })
+
+#define OR_RETURN_FATAL(expr) OR_RETURN_ERROR(Fatal, expr)
+#define OR_RETURN_NON_FATAL(expr) OR_RETURN_ERROR(NonFatal, expr)
+
ScopedAStatus Artd::isAlive(bool* _aidl_return) {
*_aidl_return = true;
return ScopedAStatus::ok();
}
ScopedAStatus Artd::deleteArtifacts(const ArtifactsPath& in_artifactsPath, int64_t* _aidl_return) {
- Result<std::string> oat_path = BuildOatPath(in_artifactsPath);
- if (!oat_path.ok()) {
- return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_STATE,
- oat_path.error().message().c_str());
- }
+ std::string oat_path = OR_RETURN_FATAL(BuildOatPath(in_artifactsPath));
*_aidl_return = 0;
- *_aidl_return += GetSizeAndDeleteFile(*oat_path);
- *_aidl_return += GetSizeAndDeleteFile(OatPathToVdexPath(*oat_path));
- *_aidl_return += GetSizeAndDeleteFile(OatPathToArtPath(*oat_path));
+ *_aidl_return += GetSizeAndDeleteFile(oat_path);
+ *_aidl_return += GetSizeAndDeleteFile(OatPathToVdexPath(oat_path));
+ *_aidl_return += GetSizeAndDeleteFile(OatPathToArtPath(oat_path));
return ScopedAStatus::ok();
}
@@ -107,9 +139,7 @@
GetOptimizationStatusResult* _aidl_return) {
Result<OatFileAssistantContext*> ofa_context = GetOatFileAssistantContext();
if (!ofa_context.ok()) {
- return ScopedAStatus::fromExceptionCodeWithMessage(
- EX_ILLEGAL_STATE,
- ("Failed to get OatFileAssistantContext: " + ofa_context.error().message()).c_str());
+ return NonFatal("Failed to get runtime options: " + ofa_context.error().message());
}
std::unique_ptr<ClassLoaderContext> context;
@@ -123,8 +153,7 @@
&context,
&error_msg);
if (oat_file_assistant == nullptr) {
- return ScopedAStatus::fromExceptionCodeWithMessage(
- EX_ILLEGAL_STATE, ("Failed to create OatFileAssistant: " + error_msg).c_str());
+ return NonFatal("Failed to create OatFileAssistant: " + error_msg);
}
std::string ignored_odex_status;
diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl
index a1df266..5cf23b9 100644
--- a/artd/binder/com/android/server/art/IArtd.aidl
+++ b/artd/binder/com/android/server/art/IArtd.aidl
@@ -21,10 +21,18 @@
// Test to see if the artd service is available.
boolean isAlive();
- /** Deletes artifacts and returns the released space, in bytes. */
+ /**
+ * Deletes artifacts and returns the released space, in bytes.
+ *
+ * Throws fatal errors. Logs and ignores non-fatal errors.
+ */
long deleteArtifacts(in com.android.server.art.ArtifactsPath artifactsPath);
- /** Returns the optimization status of a dex file. */
+ /**
+ * Returns the optimization status of a dex file.
+ *
+ * Throws fatal and non-fatal errors.
+ */
com.android.server.art.GetOptimizationStatusResult getOptimizationStatus(
@utf8InCpp String dexFile, @utf8InCpp String instructionSet,
@utf8InCpp String classLoaderContext);
diff --git a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
index 3a6bdc9..e7e011d 100644
--- a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
+++ b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
@@ -28,6 +28,7 @@
import android.os.ParcelFileDescriptor;
import android.os.RemoteException;
import android.os.ServiceManager;
+import android.os.ServiceSpecificException;
import android.util.Log;
import com.android.internal.annotations.VisibleForTesting;
@@ -187,12 +188,17 @@
continue;
}
for (String isa : Utils.getAllIsas(pkgState)) {
- GetOptimizationStatusResult result =
- mInjector.getArtd().getOptimizationStatus(
- dexInfo.dexPath(), isa, dexInfo.classLoaderContext());
- statuses.add(new DexFileOptimizationStatus(dexInfo.dexPath(), isa,
- result.compilerFilter, result.compilationReason,
- result.locationDebugString));
+ try {
+ GetOptimizationStatusResult result =
+ mInjector.getArtd().getOptimizationStatus(
+ dexInfo.dexPath(), isa, dexInfo.classLoaderContext());
+ statuses.add(new DexFileOptimizationStatus(dexInfo.dexPath(), isa,
+ result.compilerFilter, result.compilationReason,
+ result.locationDebugString));
+ } catch (ServiceSpecificException e) {
+ statuses.add(new DexFileOptimizationStatus(
+ dexInfo.dexPath(), isa, "error", "error", e.getMessage()));
+ }
}
}
}
diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
index 0e958ba..78843e3 100644
--- a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
+++ b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
@@ -31,6 +31,7 @@
import static org.mockito.Mockito.when;
import android.content.pm.ApplicationInfo;
+import android.os.ServiceSpecificException;
import androidx.test.filters.SmallTest;
@@ -188,6 +189,24 @@
mArtManagerLocal.getOptimizationStatus(mock(PackageDataSnapshot.class), PKG_NAME);
}
+ @Test
+ public void testGetOptimizationStatusNonFatalError() throws Exception {
+ when(mArtd.getOptimizationStatus(any(), any(), any()))
+ .thenThrow(new ServiceSpecificException(1 /* errorCode */, "some error message"));
+
+ OptimizationStatus result =
+ mArtManagerLocal.getOptimizationStatus(mock(PackageDataSnapshot.class), PKG_NAME);
+
+ List<DexFileOptimizationStatus> statuses = result.getDexFileOptimizationStatuses();
+ assertThat(statuses.size()).isEqualTo(4);
+
+ for (DexFileOptimizationStatus status : statuses) {
+ assertThat(status.getCompilerFilter()).isEqualTo("error");
+ assertThat(status.getCompilationReason()).isEqualTo("error");
+ assertThat(status.getLocationDebugString()).isEqualTo("some error message");
+ }
+ }
+
private AndroidPackageApi createPackage() {
AndroidPackageApi pkg = mock(AndroidPackageApi.class);