HealthMonitor: improve watchdog interface

Test: build
Change-Id: I922acacb99c58955d4d1eaaafce337d72dc4eb5d

2 files changed, 194 insertions, 8 deletions

diff --git a/base/HealthMonitor_unittest.cpp b/base/HealthMonitor_unittest.cpp
index 7001916..65ab6ca 100644
--- a/base/HealthMonitor_unittest.cpp
+++ b/base/HealthMonitor_unittest.cpp
@@ -18,9 +18,11 @@
 #include "base/HealthMonitor.h"
 
 #include <chrono>
+#include <limits>
+#include <vector>
 
-#include "base/Metrics.h"
 #include "TestClock.h"
+#include "base/Metrics.h"
 
 namespace emugl {
 
@@ -35,15 +37,23 @@ using ::testing::_;
 using ::testing::AllOf;
 using ::testing::ByMove;
 using ::testing::Contains;
+using ::testing::DoAll;
+using ::testing::Eq;
 using ::testing::Field;
 using ::testing::Ge;
 using ::testing::HasSubstr;
 using ::testing::InSequence;
+using ::testing::IsNull;
 using ::testing::Key;
 using ::testing::Le;
 using ::testing::MockFunction;
+using ::testing::Ne;
+using ::testing::Pair;
 using ::testing::Pointee;
+using ::testing::Ref;
 using ::testing::Return;
+using ::testing::SaveArg;
+using ::testing::StrEq;
 using ::testing::Test;
 using ::testing::VariantWith;
 
@@ -456,4 +466,122 @@ TEST_F(HealthMonitorTest, siblingsHangParentStillHealthy) {
     healthMonitor.stopMonitoringTask(parent);
 }
 
+class MockHealthMonitor {
+   public:
+    using Id = uint32_t;
+    MOCK_METHOD(
+        Id, startMonitoringTask,
+        (std::unique_ptr<EventHangMetadata> metadata,
+         std::optional<std::function<std::unique_ptr<HangAnnotations>()>> onHangAnnotationsCallback,
+         uint64_t timeout, std::optional<Id>));
+
+    MOCK_METHOD(void, touchMonitoredTask, (Id));
+    MOCK_METHOD(void, stopMonitoringTask, (Id));
+};
+
+TEST(HealthMonitorWatchdogBuilderTest, SimpleBuildTest) {
+    // Test simple build function and default values.
+    MockHealthMonitor monitor;
+    MockHealthMonitor::Id taskId = 0x8261;
+
+    const char message[] = "test message";
+    const int lineLowerBound = __LINE__;
+    auto builder = WATCHDOG_BUILDER(monitor, message);
+    const int lineUpperBound = __LINE__;
+    auto metadataMatcher = AllOf(
+        Pointee(Field(&EventHangMetadata::file, StrEq(__FILE__))),
+        Pointee(Field(&EventHangMetadata::function, StrEq(__func__))),
+        Pointee(Field(&EventHangMetadata::msg, StrEq(message))),
+        Pointee(Field(&EventHangMetadata::line, AllOf(Ge(lineLowerBound), Le(lineUpperBound)))),
+        Pointee(Field(&EventHangMetadata::threadId, android::base::getCurrentThreadId())),
+        Pointee(Field(&EventHangMetadata::data, IsNull())),
+        Pointee(Field(&EventHangMetadata::hangType, EventHangMetadata::HangType::kOther)));
+    EXPECT_CALL(monitor,
+                startMonitoringTask(metadataMatcher, Eq(std::nullopt), kDefaultTimeoutMs, _))
+        .Times(1)
+        .WillOnce(Return(taskId));
+    EXPECT_CALL(monitor, stopMonitoringTask(taskId)).Times(1);
+    builder.build();
+}
+
+// Test different setters.
+TEST(HealthMonitorWatchdogBuilderTest, HangTypeTest) {
+    MockHealthMonitor monitor;
+    MockHealthMonitor::Id taskId = 0x7213;
+
+    auto hangType = EventHangMetadata::HangType::kRenderThread;
+    EXPECT_CALL(monitor, startMonitoringTask(Pointee(Field(&EventHangMetadata::hangType, hangType)),
+                                             Eq(std::nullopt), kDefaultTimeoutMs, _))
+        .Times(1)
+        .WillOnce(Return(taskId));
+    EXPECT_CALL(monitor, stopMonitoringTask(taskId)).Times(1);
+    WATCHDOG_BUILDER(monitor, "test message")
+        .setHangType(EventHangMetadata::HangType::kRenderThread)
+        .build();
+}
+
+TEST(HealthMonitorWatchdogBuilderTest, TimeoutTest) {
+    MockHealthMonitor monitor;
+    MockHealthMonitor::Id taskId = 0x8749;
+    uint32_t timeoutMs = 5483;
+
+    EXPECT_CALL(monitor, startMonitoringTask(_, Eq(std::nullopt), timeoutMs, _))
+        .Times(1)
+        .WillOnce(Return(taskId));
+    EXPECT_CALL(monitor, stopMonitoringTask(taskId)).Times(1);
+    WATCHDOG_BUILDER(monitor, "test message").setTimeoutMs(timeoutMs).build();
+}
+
+TEST(HealthMonitorWatchdogBuilderTest, OnHangCallbackTest) {
+    MockHealthMonitor monitor;
+    MockHealthMonitor::Id taskId = 0x2810;
+
+    MockFunction<std::unique_ptr<HangAnnotations>()> mockOnHangCallback;
+    std::optional<std::function<std::unique_ptr<HangAnnotations>()>> actualOnHangCallback;
+
+    EXPECT_CALL(monitor, startMonitoringTask(_, Ne(std::nullopt), kDefaultTimeoutMs, _))
+        .Times(1)
+        .WillOnce(DoAll(SaveArg<1>(&actualOnHangCallback), Return(taskId)));
+    EXPECT_CALL(monitor, stopMonitoringTask(taskId)).Times(1);
+    WATCHDOG_BUILDER(monitor, "test message")
+        .setOnHangCallback(mockOnHangCallback.AsStdFunction())
+        .build();
+    EXPECT_CALL(mockOnHangCallback, Call()).Times(1);
+    (*actualOnHangCallback)();
+}
+
+TEST(HealthMonitorWatchdogBuilderTest, AnnotationsTest) {
+    MockHealthMonitor monitor;
+    MockHealthMonitor::Id taskId = 0x9271;
+
+    const char tag[] = "abcxyzalwi1943====";
+    auto annotations = std::make_unique<HangAnnotations>();
+    annotations->insert({{"tag", tag}});
+
+    EXPECT_CALL(monitor, startMonitoringTask(
+                             Pointee(Field(&EventHangMetadata::data,
+                                           Pointee(Contains(Pair(StrEq("tag"), StrEq(tag)))))),
+                             Eq(std::nullopt), kDefaultTimeoutMs, _))
+        .Times(1)
+        .WillOnce(Return(taskId));
+    EXPECT_CALL(monitor, stopMonitoringTask(taskId)).Times(1);
+    WATCHDOG_BUILDER(monitor, "test message").setAnnotations(std::move(annotations)).build();
+}
+
+TEST(HealthMonitorWatchdogBuilderTest, MultipleSettersTest) {
+    // Set multiple fields with chaining.
+    MockHealthMonitor monitor;
+    MockHealthMonitor::Id taskId = 0x9271;
+
+    uint32_t timeoutMs = 5483;
+    auto hangType = EventHangMetadata::HangType::kSyncThread;
+
+    EXPECT_CALL(monitor, startMonitoringTask(Pointee(Field(&EventHangMetadata::hangType, hangType)),
+                                             Eq(std::nullopt), timeoutMs, _))
+        .Times(1)
+        .WillOnce(Return(taskId));
+    EXPECT_CALL(monitor, stopMonitoringTask(taskId)).Times(1);
+    WATCHDOG_BUILDER(monitor, "test message").setHangType(hangType).setTimeoutMs(timeoutMs).build();
+}
+
 }  // namespace emugl
diff --git a/base/include/base/HealthMonitor.h b/base/include/base/HealthMonitor.h
index 16b9228..5587d8d 100644
--- a/base/include/base/HealthMonitor.h
+++ b/base/include/base/HealthMonitor.h
@@ -22,6 +22,7 @@
 #include <queue>

 #include <stack>

 #include <string>

+#include <type_traits>

 #include <unordered_map>

 #include <unordered_set>

 #include <variant>

@@ -36,8 +37,9 @@
 using android::base::EventHangMetadata;

 using android::base::getCurrentThreadId;

 

-#define WATCHDOG_DATA(msg, hangType, data) \

-    std::make_unique<EventHangMetadata>(__FILE__, __func__, msg, __LINE__, hangType, data)

+#define WATCHDOG_BUILDER(healthMonitor, msg)                                                       \

+    ::emugl::HealthWatchdogBuilder<std::decay_t<decltype(healthMonitor)>>(healthMonitor, __FILE__, \

+                                                                          __func__, msg, __LINE__)

 

 namespace emugl {

 

@@ -169,10 +171,13 @@ class HealthMonitor : public android::base::Thread {
 };

 

 // This class provides an RAII mechanism for monitoring a task.

-template <class Clock = steady_clock>

+// HealthMonitorT should have the exact same interface as HealthMonitor. Note that HealthWatchdog

+// can be used in performance critical path, so we use a template to dispatch a call here to

+// overcome the performance cost of virtual function dispatch.

+template <class HealthMonitorT = HealthMonitor<>>

 class HealthWatchdog {

    public:

-    HealthWatchdog(HealthMonitor<Clock>& healthMonitor, std::unique_ptr<EventHangMetadata> metadata,

+    HealthWatchdog(HealthMonitorT& healthMonitor, std::unique_ptr<EventHangMetadata> metadata,

                    std::optional<std::function<std::unique_ptr<HangAnnotations>()>>

                        onHangAnnotationsCallback = std::nullopt,

                    uint64_t timeout = kDefaultTimeoutMs)

@@ -213,9 +218,9 @@ class HealthWatchdog {
 

    private:

     using ThreadTasks =

-        std::unordered_map<HealthMonitor<Clock>*, std::stack<typename HealthMonitor<Clock>::Id>>;

-    typename HealthMonitor<Clock>::Id mId;

-    HealthMonitor<Clock>& mHealthMonitor;

+        std::unordered_map<HealthMonitorT*, std::stack<typename HealthMonitorT::Id>>;

+    typename HealthMonitorT::Id mId;

+    HealthMonitorT& mHealthMonitor;

     const unsigned long mThreadId;

 

     // Thread local stack of task Ids enables better reentrant behavior.

@@ -227,4 +232,57 @@ class HealthWatchdog {
     }

 };

 

+// HealthMonitorT should have the exact same interface as HealthMonitor. This template parameter is

+// used for injecting a different type for testing.

+template <class HealthMonitorT>

+class HealthWatchdogBuilder {

+   public:

+    HealthWatchdogBuilder(HealthMonitorT& healthMonitor, const char* fileName,

+                          const char* functionName, const char* message, uint32_t line)

+        : mHealthMonitor(healthMonitor),

+          mMetadata(std::make_unique<EventHangMetadata>(

+              fileName, functionName, message, line, EventHangMetadata::HangType::kOther, nullptr)),

+          mTimeoutMs(kDefaultTimeoutMs),

+          mOnHangCallback(std::nullopt) {}

+

+    DISALLOW_COPY_ASSIGN_AND_MOVE(HealthWatchdogBuilder);

+

+    HealthWatchdogBuilder& setHangType(EventHangMetadata::HangType hangType) {

+        mMetadata->hangType = hangType;

+        return *this;

+    }

+    HealthWatchdogBuilder& setTimeoutMs(uint32_t timeoutMs) {

+        mTimeoutMs = timeoutMs;

+        return *this;

+    }

+    // F should be a callable that returns a std::unique_ptr<EventHangMetadata::HangAnnotations>. We

+    // use template instead of std::function here to avoid extra copy.

+    template <class F>

+    HealthWatchdogBuilder& setOnHangCallback(F&& callback) {

+        mOnHangCallback =

+            std::function<std::unique_ptr<HangAnnotations>()>(std::forward<F>(callback));

+        return *this;

+    }

+

+    HealthWatchdogBuilder& setAnnotations(std::unique_ptr<HangAnnotations> annotations) {

+        mMetadata->data = std::move(annotations);

+        return *this;

+    }

+

+    std::unique_ptr<HealthWatchdog<HealthMonitorT>> build() {

+        // We are allocating on the heap, so there is a performance hit. However we also allocate

+        // EventHangMetadata on the heap, so this should be Ok. If we see performance issues with

+        // these allocations, for HealthWatchdog, we can always use placement new + noop deleter to

+        // avoid heap allocation for HealthWatchdog.

+        return std::make_unique<HealthWatchdog<HealthMonitorT>>(

+            mHealthMonitor, std::move(mMetadata), std::move(mOnHangCallback), mTimeoutMs);

+    }

+

+   private:

+    HealthMonitorT& mHealthMonitor;

+    std::unique_ptr<EventHangMetadata> mMetadata;

+    uint32_t mTimeoutMs;

+    std::optional<std::function<std::unique_ptr<HangAnnotations>()>> mOnHangCallback;

+};

+

 }  // namespace emugl