commit d34e965b387cb15a91fc44a475968b47fb740960
author Michael Butler <butlermichael@google.com> Wed Jul 03 14:26:57 2019 -0700
committer Michael Butler <butlermichael@google.com> Wed Jul 03 15:21:01 2019 -0700
tree 18c05ae048381376ada51d016f6ab31dce03da7b
parent 8ed448037a3538a9a10f6820ce425339438d1620

Check HIDL ICallback return values in sample drivers

If a Return<T> containing a transport error is not checked (isOk())
before it is destroyed, the application will be terminated with a HIDL
error. This CL checks all HIDL return values with isOk() in the driver,
and logs any appropriate message.

Bug: 118636733
Test: mma
Test: NeuralNetworksTest_static (with sample drivers)
Change-Id: I9b2547b7a6a76bd2c97f535ae55c38725dc6aed9

driver/sample/SampleDriver.cpp

diff --git a/driver/sample/SampleDriver.cpp b/driver/sample/SampleDriver.cpp
index 25955e7..d8c0ee5 100644
--- a/driver/sample/SampleDriver.cpp
+++ b/driver/sample/SampleDriver.cpp
@@ -124,12 +124,19 @@
 
 static void notify(const sp<V1_0::IPreparedModelCallback>& callback, const ErrorStatus& status,
                    const sp<SamplePreparedModel>& preparedModel) {
-    callback->notify(status, preparedModel);
+    const auto ret = callback->notify(status, preparedModel);
+    if (!ret.isOk()) {
+        LOG(ERROR) << "Error when calling IPreparedModelCallback::notify: " << ret.description();
+    }
 }
 
 static void notify(const sp<V1_2::IPreparedModelCallback>& callback, const ErrorStatus& status,
                    const sp<SamplePreparedModel>& preparedModel) {
-    callback->notify_1_2(status, preparedModel);
+    const auto ret = callback->notify_1_2(status, preparedModel);
+    if (!ret.isOk()) {
+        LOG(ERROR) << "Error when calling IPreparedModelCallback::notify_1_2: "
+                   << ret.description();
+    }
 }
 
 template <typename T_Model, typename T_IPreparedModelCallback>
@@ -185,7 +192,7 @@
         const sp<V1_2::IPreparedModelCallback>& callback) {
     NNTRACE_FULL(NNTRACE_LAYER_DRIVER, NNTRACE_PHASE_COMPILATION,
                  "SampleDriver::prepareModelFromCache");
-    callback->notify_1_2(ErrorStatus::GENERAL_FAILURE, nullptr);
+    notify(callback, ErrorStatus::GENERAL_FAILURE, nullptr);
     return ErrorStatus::GENERAL_FAILURE;
 }
 
@@ -211,14 +218,20 @@
     return setRunTimePoolInfosFromHidlMemories(&mPoolInfos, mModel.pools);
 }
 
-static Return<void> notify(const sp<V1_0::IExecutionCallback>& callback, const ErrorStatus& status,
-                           const hidl_vec<OutputShape>&, Timing) {
-    return callback->notify(status);
+static void notify(const sp<V1_0::IExecutionCallback>& callback, const ErrorStatus& status,
+                   const hidl_vec<OutputShape>&, Timing) {
+    const auto ret = callback->notify(status);
+    if (!ret.isOk()) {
+        LOG(ERROR) << "Error when calling IExecutionCallback::notify: " << ret.description();
+    }
 }
 
-static Return<void> notify(const sp<V1_2::IExecutionCallback>& callback, const ErrorStatus& status,
-                           const hidl_vec<OutputShape>& outputShapes, Timing timing) {
-    return callback->notify_1_2(status, outputShapes, timing);
+static void notify(const sp<V1_2::IExecutionCallback>& callback, const ErrorStatus& status,
+                   const hidl_vec<OutputShape>& outputShapes, Timing timing) {
+    const auto ret = callback->notify_1_2(status, outputShapes, timing);
+    if (!ret.isOk()) {
+        LOG(ERROR) << "Error when calling IExecutionCallback::notify_1_2: " << ret.description();
+    }
 }
 
 // TODO(xusongw): Let callback notify actual output shape once dynamic output shape
@@ -246,18 +259,14 @@
     VLOG(DRIVER) << "executor.run returned " << n;
     ErrorStatus executionStatus = convertResultCodeToErrorStatus(n);
     hidl_vec<OutputShape> outputShapes = executor.getOutputShapes();
-    Return<void> returned;
     if (measure == MeasureTiming::YES && executionStatus == ErrorStatus::NONE) {
         driverEnd = now();
         Timing timing = {.timeOnDevice = uint64_t(microsecondsDuration(deviceEnd, deviceStart)),
                          .timeInDriver = uint64_t(microsecondsDuration(driverEnd, driverStart))};
         VLOG(DRIVER) << "SampleDriver::asyncExecute timing = " << toString(timing);
-        returned = notify(callback, executionStatus, outputShapes, timing);
+        notify(callback, executionStatus, outputShapes, timing);
     } else {
-        returned = notify(callback, executionStatus, outputShapes, kNoTiming);
-    }
-    if (!returned.isOk()) {
-        LOG(ERROR) << " hidl callback failed to return properly: " << returned.description();
+        notify(callback, executionStatus, outputShapes, kNoTiming);
     }
 }