commit eda1f923a4d9199a98e20e408c9e904b8a552825
author Steven Moreland <smoreland@google.com> Tue Mar 05 19:37:51 2019 -0800
committer Steven Moreland <smoreland@google.com> Wed Mar 06 21:47:34 2019 +0000
tree 877cc540775035c26a43790a05e66d716f2e95ec
parent 779a714661416fa930c5a2b39efc4372327f7fc3

Add setProcessHidlReturnRestriction.

For critical processes (e.g. init/hwservicemanager), it's nice to check
that every error is checked. If these processes restart, it may be hard
to debug the system.

Bug: 124861676
Bug: 121004730
Test: use w/ hwservicemanager
Change-Id: I0d340c31e392bfb86a188dab902e6d20fa836814
Merged-In: I0d340c31e392bfb86a188dab902e6d20fa836814

base/Status.cpp

diff --git a/base/Status.cpp b/base/Status.cpp
index 5a4c918..90474a0 100644
--- a/base/Status.cpp
+++ b/base/Status.cpp
@@ -18,6 +18,7 @@
 #include <android-base/logging.h>
 
 #include <hidl/Status.h>
+#include <utils/CallStack.h>
 
 #include <unordered_map>
 
@@ -142,6 +143,11 @@
     return stream;
 }
 
+static HidlReturnRestriction gReturnRestriction = HidlReturnRestriction::NONE;
+void setProcessHidlReturnRestriction(HidlReturnRestriction restriction) {
+    gReturnRestriction = restriction;
+}
+
 namespace details {
     void return_status::assertOk() const {
         if (!isOk()) {
@@ -151,9 +157,22 @@
 
     return_status::~return_status() {
         // mCheckedStatus must be checked before isOk since isOk modifies mCheckedStatus
-        if (!mCheckedStatus && !isOk()) {
+        if (mCheckedStatus) return;
+
+        if (!isOk()) {
             LOG(FATAL) << "Failed HIDL return status not checked: " << description();
         }
+
+        if (gReturnRestriction == HidlReturnRestriction::NONE) {
+            return;
+        }
+
+        if (gReturnRestriction == HidlReturnRestriction::ERROR_IF_UNCHECKED) {
+            LOG(ERROR) << "Failed to check status of HIDL Return.";
+            CallStack::logStack("unchecked HIDL return", CallStack::getCurrent(10).get(), ANDROID_LOG_ERROR);
+        } else {
+            LOG(FATAL) << "Failed to check status of HIDL Return.";
+        }
     }
 
     return_status& return_status::operator=(return_status&& other) noexcept {

base/include/hidl/Status.h

diff --git a/base/include/hidl/Status.h b/base/include/hidl/Status.h
index 765f0f7..817277f 100644
--- a/base/include/hidl/Status.h
+++ b/base/include/hidl/Status.h
@@ -143,9 +143,8 @@
 
         template <typename T, typename U>
         friend Return<U> StatusOf(const Return<T> &other);
-    protected:
-        void assertOk() const;
     public:
+        void assertOk() const;
         return_status() {}
         return_status(const Status& s) : mStatus(s) {}
 
@@ -189,6 +188,26 @@
     };
 }  // namespace details
 
+enum class HidlReturnRestriction {
+    // Okay to ignore checking transport errors. This would instead rely on init to reset state
+    // after an error in the underlying transport. This is the default and expected for most
+    // usecases.
+    NONE,
+    // Log when there is an unchecked error.
+    ERROR_IF_UNCHECKED,
+    // Fatal when there is an unchecked error.
+    FATAL_IF_UNCHECKED,
+};
+
+/**
+ * This should be called during process initialization (e.g. before binder threadpool is created).
+ *
+ * Note: default of HidlReturnRestriction::NONE should be good for most usecases. See above.
+ *
+ * The restriction will be applied when Return objects are deconstructed.
+ */
+void setProcessHidlReturnRestriction(HidlReturnRestriction restriction);
+
 template<typename T> class Return : public details::return_status {
 private:
     T mVal {};