From 9f439397644906180f6d2a2be6f96069d05daf78 Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Fri, 21 Aug 2020 15:09:59 +0000
Subject: [PATCH 01/11] init implementation of abort_action.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 CODEOWNERS                                    |   1 +
 api/BUILD                                     |   1 +
 .../watchdog/abort_action/v3alpha/BUILD       |   9 +
 .../abort_action/v3alpha/abort_action.proto   |  25 +++
 api/versioning/BUILD                          |   1 +
 docs/root/api-v3/config/watchdog/watchdog.rst |   1 +
 docs/root/version_history/current.rst         |   1 +
 .../watchdog/abort_action/v3alpha/BUILD       |   9 +
 .../abort_action/v3alpha/abort_action.proto   |  25 +++
 include/envoy/thread/thread.h                 |   1 +
 source/extensions/extensions_build_config.bzl |   1 +
 source/extensions/watchdog/abort_action/BUILD |  41 +++++
 .../watchdog/abort_action/abort_action.cc     |  68 ++++++++
 .../watchdog/abort_action/abort_action.h      |  37 +++++
 .../watchdog/abort_action/config.cc           |  32 ++++
 .../extensions/watchdog/abort_action/config.h |  37 +++++
 test/extensions/watchdog/abort_action/BUILD   |  46 ++++++
 .../abort_action/abort_action_test.cc         | 155 ++++++++++++++++++
 .../watchdog/abort_action/config_test.cc      |  54 ++++++
 19 files changed, 545 insertions(+)
 create mode 100644 api/envoy/extensions/watchdog/abort_action/v3alpha/BUILD
 create mode 100644 api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
 create mode 100644 generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/BUILD
 create mode 100644 generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
 create mode 100644 source/extensions/watchdog/abort_action/BUILD
 create mode 100644 source/extensions/watchdog/abort_action/abort_action.cc
 create mode 100644 source/extensions/watchdog/abort_action/abort_action.h
 create mode 100644 source/extensions/watchdog/abort_action/config.cc
 create mode 100644 source/extensions/watchdog/abort_action/config.h
 create mode 100644 test/extensions/watchdog/abort_action/BUILD
 create mode 100644 test/extensions/watchdog/abort_action/abort_action_test.cc
 create mode 100644 test/extensions/watchdog/abort_action/config_test.cc

diff --git a/CODEOWNERS b/CODEOWNERS
index 1edcf7c68c1e..bf8df7415340 100644
--- a/CODEOWNERS
+++ b/CODEOWNERS
@@ -130,6 +130,7 @@ extensions/filters/common/original_src @snowp @klarose
 /*/extensions/filters/http/decompressor @rojkov @dio
 # Watchdog Extensions
 /*/extensions/watchdog/profile_action @kbaichoo @htuch
+/*/extensions/watchdog/abort_action @kbaichoo @htuch
 # Core upstream code
 extensions/upstreams/http @alyssawilk @snowp @mattklein123
 extensions/upstreams/http/http @alyssawilk @snowp @mattklein123
diff --git a/api/BUILD b/api/BUILD
index 4aad4899a847..d981622ec1b4 100644
--- a/api/BUILD
+++ b/api/BUILD
@@ -245,6 +245,7 @@ proto_library(
         "//envoy/extensions/upstreams/http/http/v3:pkg",
         "//envoy/extensions/upstreams/http/tcp/v3:pkg",
         "//envoy/extensions/wasm/v3:pkg",
+        "//envoy/extensions/watchdog/abort_action/v3alpha:pkg",
         "//envoy/extensions/watchdog/profile_action/v3alpha:pkg",
         "//envoy/service/accesslog/v3:pkg",
         "//envoy/service/auth/v3:pkg",
diff --git a/api/envoy/extensions/watchdog/abort_action/v3alpha/BUILD b/api/envoy/extensions/watchdog/abort_action/v3alpha/BUILD
new file mode 100644
index 000000000000..ee92fb652582
--- /dev/null
+++ b/api/envoy/extensions/watchdog/abort_action/v3alpha/BUILD
@@ -0,0 +1,9 @@
+# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py.
+
+load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")
+
+licenses(["notice"])  # Apache 2
+
+api_proto_package(
+    deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
+)
diff --git a/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto b/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
new file mode 100644
index 000000000000..e659444e429d
--- /dev/null
+++ b/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
@@ -0,0 +1,25 @@
+syntax = "proto3";
+
+package envoy.extensions.watchdog.abort_action.v3alpha;
+
+import "google/protobuf/duration.proto";
+
+import "udpa/annotations/status.proto";
+import "udpa/annotations/versioning.proto";
+import "validate/validate.proto";
+
+option java_package = "io.envoyproxy.envoy.extensions.watchdog.abort_action.v3alpha";
+option java_outer_classname = "AbortActionProto";
+option java_multiple_files = true;
+option (udpa.annotations.file_status).work_in_progress = true;
+option (udpa.annotations.file_status).package_version_status = ACTIVE;
+
+// [#protodoc-title: Watchdog Action that does CPU profiling.]
+// [#extension: envoy.watchdog.abort_action]
+
+// Configuration for the profile watchdog action.
+message AbortActionConfig {
+  // How long to wait for the thread to respond to the abort before killing the
+  // process from this action. This is a blocking action.
+  google.protobuf.Duration wait_duration = 1;
+}
diff --git a/api/versioning/BUILD b/api/versioning/BUILD
index c93b1f7d84c5..48c8df7fdf92 100644
--- a/api/versioning/BUILD
+++ b/api/versioning/BUILD
@@ -128,6 +128,7 @@ proto_library(
         "//envoy/extensions/upstreams/http/http/v3:pkg",
         "//envoy/extensions/upstreams/http/tcp/v3:pkg",
         "//envoy/extensions/wasm/v3:pkg",
+        "//envoy/extensions/watchdog/abort_action/v3alpha:pkg",
         "//envoy/extensions/watchdog/profile_action/v3alpha:pkg",
         "//envoy/service/accesslog/v3:pkg",
         "//envoy/service/auth/v3:pkg",
diff --git a/docs/root/api-v3/config/watchdog/watchdog.rst b/docs/root/api-v3/config/watchdog/watchdog.rst
index 60f284384d59..f3fd56c327ec 100644
--- a/docs/root/api-v3/config/watchdog/watchdog.rst
+++ b/docs/root/api-v3/config/watchdog/watchdog.rst
@@ -6,3 +6,4 @@ Watchdog
   :maxdepth: 2
 
   ../../extensions/watchdog/profile_action/v3alpha/*
+  ../../extensions/watchdog/abort_action/v3alpha/*
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
index 6b5a94162552..06c813204c33 100644
--- a/docs/root/version_history/current.rst
+++ b/docs/root/version_history/current.rst
@@ -100,6 +100,7 @@ New Features
 * watchdog: support randomizing the watchdog's kill timeout to prevent synchronized kills via a maximium jitter parameter :ref:`max_kill_timeout_jitter<envoy_v3_api_field_config.bootstrap.v3.Watchdog.max_kill_timeout_jitter>`.
 * watchdog: supports an extension point where actions can be registered to fire on watchdog events such as miss, megamiss, kill and multikill. See ref:`watchdog actions<envoy_v3_api_field_config.bootstrap.v3.Watchdog.actions>`.
 * watchdog: watchdog action extension that does cpu profiling. See ref:`Profile Action <envoy_v3_api_file_envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto>`.
+* watchdog: watchdog action extension that signals an abort to a particular thread to kill the process. See ref:`Abort Action <envoy_v3_api_file_envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto>`.
 * xds: added :ref:`extension config discovery<envoy_v3_api_msg_config.core.v3.ExtensionConfigSource>` support for HTTP filters.
 * zlib: added option to use `zlib-ng <https://github.com/zlib-ng/zlib-ng>`_ as zlib library.
 
diff --git a/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/BUILD b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/BUILD
new file mode 100644
index 000000000000..ee92fb652582
--- /dev/null
+++ b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/BUILD
@@ -0,0 +1,9 @@
+# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py.
+
+load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")
+
+licenses(["notice"])  # Apache 2
+
+api_proto_package(
+    deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
+)
diff --git a/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
new file mode 100644
index 000000000000..e659444e429d
--- /dev/null
+++ b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
@@ -0,0 +1,25 @@
+syntax = "proto3";
+
+package envoy.extensions.watchdog.abort_action.v3alpha;
+
+import "google/protobuf/duration.proto";
+
+import "udpa/annotations/status.proto";
+import "udpa/annotations/versioning.proto";
+import "validate/validate.proto";
+
+option java_package = "io.envoyproxy.envoy.extensions.watchdog.abort_action.v3alpha";
+option java_outer_classname = "AbortActionProto";
+option java_multiple_files = true;
+option (udpa.annotations.file_status).work_in_progress = true;
+option (udpa.annotations.file_status).package_version_status = ACTIVE;
+
+// [#protodoc-title: Watchdog Action that does CPU profiling.]
+// [#extension: envoy.watchdog.abort_action]
+
+// Configuration for the profile watchdog action.
+message AbortActionConfig {
+  // How long to wait for the thread to respond to the abort before killing the
+  // process from this action. This is a blocking action.
+  google.protobuf.Duration wait_duration = 1;
+}
diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h
index bcc6864d1466..aacc53d997fa 100644
--- a/include/envoy/thread/thread.h
+++ b/include/envoy/thread/thread.h
@@ -24,6 +24,7 @@ class ThreadId {
   explicit ThreadId(int64_t id) : id_(id) {}
 
   std::string debugString() const { return std::to_string(id_); }
+  int64_t getId() const { return id_; }
   bool isEmpty() const { return *this == ThreadId(); }
   friend bool operator==(ThreadId lhs, ThreadId rhs) { return lhs.id_ == rhs.id_; }
   friend bool operator!=(ThreadId lhs, ThreadId rhs) { return lhs.id_ != rhs.id_; }
diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl
index 6b4929e0dd59..00cbe6d4c8d0 100644
--- a/source/extensions/extensions_build_config.bzl
+++ b/source/extensions/extensions_build_config.bzl
@@ -203,6 +203,7 @@ EXTENSIONS = {
     # Watchdog actions
     #
     "envoy.watchdog.profile_action":                    "//source/extensions/watchdog/profile_action:config",
+    "envoy.watchdog.abort_action":                      "//source/extensions/watchdog/abort_action:config",
 
 }
 
diff --git a/source/extensions/watchdog/abort_action/BUILD b/source/extensions/watchdog/abort_action/BUILD
new file mode 100644
index 000000000000..21d1a4304a06
--- /dev/null
+++ b/source/extensions/watchdog/abort_action/BUILD
@@ -0,0 +1,41 @@
+load(
+    "//bazel:envoy_build_system.bzl",
+    "envoy_cc_extension",
+    "envoy_cc_library",
+    "envoy_extension_package",
+)
+
+licenses(["notice"])  # Apache 2
+
+envoy_extension_package()
+
+envoy_cc_library(
+    name = "abort_action_lib",
+    srcs = ["abort_action.cc"],
+    hdrs = ["abort_action.h"],
+    deps = [
+        "//include/envoy/common:time_interface",
+        "//include/envoy/server:guarddog_config_interface",
+        "//include/envoy/thread:thread_interface",
+        "//source/common/common:assert_lib",
+        "//source/common/protobuf:utility_lib",
+        "@envoy_api//envoy/extensions/watchdog/abort_action/v3alpha:pkg_cc_proto",
+    ],
+)
+
+envoy_cc_extension(
+    name = "config",
+    srcs = ["config.cc"],
+    hdrs = ["config.h"],
+    security_posture = "robust_to_untrusted_downstream_and_upstream",
+    status = "alpha",
+    deps = [
+        ":abort_action_lib",
+        "//include/envoy/registry",
+        "//source/common/config:utility_lib",
+        "//source/common/protobuf",
+        "//source/common/protobuf:message_validator_lib",
+        "//source/extensions/watchdog:well_known_names",
+        "@envoy_api//envoy/extensions/watchdog/abort_action/v3alpha:pkg_cc_proto",
+    ],
+)
diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
new file mode 100644
index 000000000000..30296e1b5bad
--- /dev/null
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -0,0 +1,68 @@
+#include "extensions/watchdog/abort_action/abort_action.h"
+
+#include "common/common/assert.h"
+#include "common/common/fmt.h"
+
+#ifndef WIN32
+#include <sys/types.h>
+#include <csignal>
+#endif
+
+#include "common/common/logger.h"
+#include "common/protobuf/utility.h"
+#include "envoy/thread/thread.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Watchdog {
+namespace AbortAction {
+namespace {
+#ifdef __linux__
+pid_t to_platform_tid(int64_t tid) { return static_cast<pid_t>(tid); }
+#elif defined(__APPLE__)
+uint64 to_platform_tid(int64_t tid) { return static_cast<uint64>(tid); }
+#endif
+} // namespace
+
+AbortAction::AbortAction(
+    envoy::extensions::watchdog::abort_action::v3alpha::AbortActionConfig& config,
+    Server::Configuration::GuardDogActionFactoryContext& /*context*/)
+    : config_(config){};
+
+void AbortAction::run(
+    envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent /*event*/,
+    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
+    MonotonicTime /*now*/) {
+
+  if (thread_ltt_pairs.empty()) {
+    ENVOY_LOG_MISC(warn, "AbortAction called without any thread.");
+    return;
+  }
+
+  int64_t raw_tid = thread_ltt_pairs.at(0).first.getId();
+
+#ifdef WIN32
+  // TODO(kbaichoo): add support for this with windows.
+  ENVOY_LOG_MISC(error, "AbortAction is unimplemented for Windows.");
+#else
+  // Assume POSIX-compatible system and signal to the thread.
+  ENVOY_LOG_MISC(error, "AbortAction sending abort signal to thread with tid {}.", raw_tid);
+
+  if (kill(to_platform_tid(raw_tid), SIGABRT) == 0) {
+    // Successfully sent signal, sleep for wait_duration.
+    absl::SleepFor(absl::Milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(config_, wait_duration, 0)));
+  } else {
+    // Failed to send the signal, abort?
+    ENVOY_LOG_MISC(error, "Failed to send signal to tid {}", raw_tid);
+  }
+
+  // Abort from the action since the signaled thread hasn't yet crashed the proccess.
+  PANIC(
+      fmt::format("Failed to kill thread with id {}, aborting from AbortAction instead.", raw_tid));
+#endif
+}
+
+} // namespace AbortAction
+} // namespace Watchdog
+} // namespace Extensions
+} // namespace Envoy
diff --git a/source/extensions/watchdog/abort_action/abort_action.h b/source/extensions/watchdog/abort_action/abort_action.h
new file mode 100644
index 000000000000..b9e03d71474c
--- /dev/null
+++ b/source/extensions/watchdog/abort_action/abort_action.h
@@ -0,0 +1,37 @@
+#pragma once
+
+#include <chrono>
+
+#include "envoy/extensions/watchdog/abort_action/v3alpha/abort_action.pb.h"
+#include "envoy/server/guarddog_config.h"
+#include "envoy/thread/thread.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Watchdog {
+namespace AbortAction {
+
+/**
+ * A GuardDogAction that will start signal to a particular thread to abort.
+ * This is currently only implemented for systems that support kill to send
+ * signals.
+ */
+class AbortAction : public Server::Configuration::GuardDogAction {
+public:
+  AbortAction(envoy::extensions::watchdog::abort_action::v3alpha::AbortActionConfig& config,
+              Server::Configuration::GuardDogActionFactoryContext& context);
+
+  void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event,
+           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
+           MonotonicTime now) override;
+
+private:
+  const envoy::extensions::watchdog::abort_action::v3alpha::AbortActionConfig config_;
+};
+
+using AbortActionPtr = std::unique_ptr<AbortAction>;
+
+} // namespace AbortAction
+} // namespace Watchdog
+} // namespace Extensions
+} // namespace Envoy
diff --git a/source/extensions/watchdog/abort_action/config.cc b/source/extensions/watchdog/abort_action/config.cc
new file mode 100644
index 000000000000..f59c62c94cf4
--- /dev/null
+++ b/source/extensions/watchdog/abort_action/config.cc
@@ -0,0 +1,32 @@
+#include "extensions/watchdog/abort_action/config.h"
+
+#include "envoy/registry/registry.h"
+
+#include "common/config/utility.h"
+#include "common/protobuf/message_validator_impl.h"
+
+#include "extensions/watchdog/abort_action/abort_action.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Watchdog {
+namespace AbortAction {
+
+Server::Configuration::GuardDogActionPtr AbortActionFactory::createGuardDogActionFromProto(
+    const envoy::config::bootstrap::v3::Watchdog::WatchdogAction& config,
+    Server::Configuration::GuardDogActionFactoryContext& context) {
+  auto message = createEmptyConfigProto();
+  Config::Utility::translateOpaqueConfig(config.config().typed_config(), ProtobufWkt::Struct(),
+                                         ProtobufMessage::getStrictValidationVisitor(), *message);
+  return std::make_unique<AbortAction>(dynamic_cast<AbortActionConfig&>(*message), context);
+}
+
+/**
+ * Static registration for the fixed heap resource monitor factory. @see RegistryFactory.
+ */
+REGISTER_FACTORY(AbortActionFactory, Server::Configuration::GuardDogActionFactory);
+
+} // namespace AbortAction
+} // namespace Watchdog
+} // namespace Extensions
+} // namespace Envoy
diff --git a/source/extensions/watchdog/abort_action/config.h b/source/extensions/watchdog/abort_action/config.h
new file mode 100644
index 000000000000..c2c7aef9c73f
--- /dev/null
+++ b/source/extensions/watchdog/abort_action/config.h
@@ -0,0 +1,37 @@
+#pragma once
+
+#include "envoy/extensions/watchdog/abort_action/v3alpha/abort_action.pb.h"
+#include "envoy/server/guarddog_config.h"
+
+#include "common/protobuf/protobuf.h"
+
+#include "extensions/watchdog/well_known_names.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Watchdog {
+namespace AbortAction {
+
+class AbortActionFactory : public Server::Configuration::GuardDogActionFactory {
+public:
+  AbortActionFactory() : name_(WatchdogActionNames::get().AbortAction){};
+
+  Server::Configuration::GuardDogActionPtr createGuardDogActionFromProto(
+      const envoy::config::bootstrap::v3::Watchdog::WatchdogAction& config,
+      Server::Configuration::GuardDogActionFactoryContext& context) override;
+
+  ProtobufTypes::MessagePtr createEmptyConfigProto() override {
+    return std::make_unique<AbortActionConfig>();
+  }
+
+  std::string name() const override { return name_; }
+
+private:
+  using AbortActionConfig = envoy::extensions::watchdog::abort_action::v3alpha::AbortActionConfig;
+  const std::string name_;
+};
+
+} // namespace AbortAction
+} // namespace Watchdog
+} // namespace Extensions
+} // namespace Envoy
diff --git a/test/extensions/watchdog/abort_action/BUILD b/test/extensions/watchdog/abort_action/BUILD
new file mode 100644
index 000000000000..1a7acd173ac7
--- /dev/null
+++ b/test/extensions/watchdog/abort_action/BUILD
@@ -0,0 +1,46 @@
+load(
+    "//bazel:envoy_build_system.bzl",
+    "envoy_package",
+)
+load(
+    "//test/extensions:extensions_build_system.bzl",
+    "envoy_extension_cc_test",
+)
+
+licenses(["notice"])  # Apache 2
+
+envoy_package()
+
+envoy_extension_cc_test(
+    name = "abort_action_test",
+    srcs = ["abort_action_test.cc"],
+    extension_name = "envoy.watchdog.abort_action",
+    external_deps = [
+        "abseil_synchronization",
+    ],
+    deps = [
+        "//include/envoy/common:time_interface",
+        "//include/envoy/registry",
+        "//include/envoy/server:guarddog_config_interface",
+        "//source/extensions/watchdog/abort_action:abort_action_lib",
+        "//source/extensions/watchdog/abort_action:config",
+        "//test/test_common:utility_lib",
+        "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
+        "@envoy_api//envoy/extensions/watchdog/abort_action/v3alpha:pkg_cc_proto",
+    ],
+)
+
+envoy_extension_cc_test(
+    name = "config_test",
+    srcs = ["config_test.cc"],
+    extension_name = "envoy.watchdog.abort_action",
+    deps = [
+        "//include/envoy/registry",
+        "//include/envoy/server:guarddog_config_interface",
+        "//source/extensions/watchdog/abort_action:abort_action_lib",
+        "//source/extensions/watchdog/abort_action:config",
+        "//test/mocks/event:event_mocks",
+        "//test/test_common:utility_lib",
+        "@envoy_api//envoy/extensions/watchdog/abort_action/v3alpha:pkg_cc_proto",
+    ],
+)
diff --git a/test/extensions/watchdog/abort_action/abort_action_test.cc b/test/extensions/watchdog/abort_action/abort_action_test.cc
new file mode 100644
index 000000000000..2ad32ed9aa2c
--- /dev/null
+++ b/test/extensions/watchdog/abort_action/abort_action_test.cc
@@ -0,0 +1,155 @@
+#include <csignal>
+#include <memory>
+
+#include "envoy/common/time.h"
+#include "envoy/config/bootstrap/v3/bootstrap.pb.h"
+#include "envoy/event/dispatcher.h"
+#include "envoy/extensions/watchdog/abort_action/v3alpha/abort_action.pb.h"
+#include "envoy/server/guarddog_config.h"
+#include "envoy/thread/thread.h"
+
+#include "extensions/watchdog/abort_action/abort_action.h"
+#include "extensions/watchdog/abort_action/config.h"
+
+#include "test/test_common/utility.h"
+
+#include "absl/synchronization/mutex.h"
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Watchdog {
+namespace AbortAction {
+namespace {
+
+using AbortActionConfig = envoy::extensions::watchdog::abort_action::v3alpha::AbortActionConfig;
+
+class AbortActionTest : public testing::Test {
+protected:
+  AbortActionTest()
+      : api_(Api::createApiForTest()), dispatcher_(api_->allocateDispatcher("test")),
+        context_({*api_, *dispatcher_}) {}
+
+  void waitForOutstandingNotify() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_) {
+    mutex_.Await(absl::Condition(
+        +[](int* outstanding_notifies) -> bool { return *outstanding_notifies > 0; },
+        &outstanding_notifies_));
+    outstanding_notifies_ -= 1;
+  }
+
+  Api::ApiPtr api_;
+  Event::DispatcherPtr dispatcher_;
+  Server::Configuration::GuardDogActionFactoryContext context_;
+  std::unique_ptr<Server::Configuration::GuardDogAction> action_;
+
+  // Used to synchronize with the dispatch thread
+  absl::Mutex mutex_;
+  int outstanding_notifies_ ABSL_GUARDED_BY(mutex_) = 0;
+};
+
+TEST_F(AbortActionTest, ShouldNotAbortIfNoTids) {
+  AbortActionConfig config;
+  config.mutable_wait_duration()->set_nanos(1000000);
+  action_ = std::make_unique<AbortAction>(config, context_);
+
+  // Create empty vector and run the action.
+  const auto now = api_->timeSource().monotonicTime();
+  const std::vector<std::pair<Thread::ThreadId, MonotonicTime>> tid_ltt_pairs = {};
+
+  // Should not signal or panic since there are no tids.
+  action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::KILL, tid_ltt_pairs, now);
+}
+
+// insufficient signal support on Windows.
+#ifndef WIN32
+TEST_F(AbortActionTest, CanKillThread) {
+  AbortActionConfig config;
+  config.mutable_wait_duration()->set_seconds(1);
+  action_ = std::make_unique<AbortAction>(config, context_);
+
+  auto die_function = [this]() -> void {
+    // Create a thread that we'll kill
+    Thread::ThreadId tid;
+    Thread::ThreadPtr thread = api_->threadFactory().createThread([this, &tid]() -> void {
+      tid = api_->threadFactory().currentThreadId();
+
+      // Signal to test thread that tid has been set.
+      {
+        absl::MutexLock lock(&mutex_);
+        outstanding_notifies_ += 1;
+      }
+
+      dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit);
+    });
+
+    // Wait for child thread to notify of its tid
+    {
+      absl::MutexLock lock(&mutex_);
+      waitForOutstandingNotify();
+    }
+
+    // Create vector with child tid and run the action.
+    const auto now = api_->timeSource().monotonicTime();
+    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>> tid_ltt_pairs = {{tid, now}};
+
+    action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::KILL, tid_ltt_pairs, now);
+  };
+
+  EXPECT_DEATH(die_function(), "Caught Aborted");
+}
+
+void handler(int sig, siginfo_t* /*siginfo*/, void* /*context*/) {
+  std::cout << "Eating signal :" << std::to_string(sig) << ". will ignore it." << std::endl;
+  signal(SIGABRT, SIG_IGN);
+}
+
+TEST_F(AbortActionTest, PanicsIfThreadDoesNotDie) {
+  AbortActionConfig config;
+  config.mutable_wait_duration()->set_seconds(1);
+  action_ = std::make_unique<AbortAction>(config, context_);
+
+  auto die_function = [this]() -> void {
+    // Create a thread that we try to kill
+    Thread::ThreadId tid;
+    Thread::ThreadPtr thread = api_->threadFactory().createThread([this, &tid]() -> void {
+      tid = api_->threadFactory().currentThreadId();
+
+      // Prepare signal handler to eat SIGABRT for the child thread.
+      struct sigaction saction;
+      std::memset(&saction, 0, sizeof(saction));
+      saction.sa_flags = SA_SIGINFO;
+      saction.sa_sigaction = &handler;
+      sigaction(SIGABRT, &saction, NULL);
+
+      // Signal to test thread that tid has been set.
+      {
+        absl::MutexLock lock(&mutex_);
+        outstanding_notifies_ += 1;
+      }
+
+      dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit);
+    });
+
+    // Wait for child thread to notify of its tid
+    {
+      absl::MutexLock lock(&mutex_);
+      waitForOutstandingNotify();
+    }
+
+    // Create vector with child tid and run the action.
+    const auto now = api_->timeSource().monotonicTime();
+    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>> tid_ltt_pairs = {{tid, now}};
+
+    action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::KILL, tid_ltt_pairs, now);
+  };
+
+  EXPECT_DEATH(die_function(), "aborting from AbortAction instead");
+}
+
+#endif
+
+} // namespace
+} // namespace AbortAction
+} // namespace Watchdog
+} // namespace Extensions
+} // namespace Envoy
diff --git a/test/extensions/watchdog/abort_action/config_test.cc b/test/extensions/watchdog/abort_action/config_test.cc
new file mode 100644
index 000000000000..b5a490118f6e
--- /dev/null
+++ b/test/extensions/watchdog/abort_action/config_test.cc
@@ -0,0 +1,54 @@
+#include "envoy/extensions/watchdog/abort_action/v3alpha/abort_action.pb.h"
+#include "envoy/registry/registry.h"
+#include "envoy/server/guarddog_config.h"
+
+#include "extensions/watchdog/abort_action/config.h"
+
+#include "test/mocks/event/mocks.h"
+#include "test/test_common/utility.h"
+
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Watchdog {
+namespace AbortAction {
+namespace {
+
+TEST(AbortActionFactoryTest, CanCreateAction) {
+  auto factory =
+      Registry::FactoryRegistry<Server::Configuration::GuardDogActionFactory>::getFactory(
+          "envoy.watchdog.abort_action");
+  ASSERT_NE(factory, nullptr);
+
+  // Create config and mock context
+  envoy::config::bootstrap::v3::Watchdog::WatchdogAction config;
+  TestUtility::loadFromJson(
+      R"EOF(
+        {
+          "config": {
+            "name": "envoy.watchdog.abort_action",
+            "typed_config": {
+	      "@type": "type.googleapis.com/udpa.type.v1.TypedStruct",
+	      "type_url": "type.googleapis.com/envoy.extensions.watchdog.abort_action.v3alpha.AbortActionConfig",
+	      "value": {
+		"wait_duration": "2s",
+	      }
+            }
+          },
+        }
+      )EOF",
+      config);
+
+  Event::MockDispatcher dispatcher;
+  Api::ApiPtr api = Api::createApiForTest();
+  Server::Configuration::GuardDogActionFactoryContext context{*api, dispatcher};
+
+  EXPECT_NE(factory->createGuardDogActionFromProto(config, context), nullptr);
+}
+
+} // namespace
+} // namespace AbortAction
+} // namespace Watchdog
+} // namespace Extensions
+} // namespace Envoy

From 015403462f90280f7c7448fb0237da8062bc8d1a Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Fri, 28 Aug 2020 01:15:09 +0000
Subject: [PATCH 02/11] Spelling, minor fixes.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 source/extensions/watchdog/abort_action/BUILD             | 1 -
 source/extensions/watchdog/abort_action/abort_action.cc   | 8 ++++----
 source/extensions/watchdog/abort_action/config.h          | 4 +---
 .../extensions/watchdog/abort_action/abort_action_test.cc | 4 ++--
 tools/spelling/spelling_dictionary.txt                    | 1 +
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/source/extensions/watchdog/abort_action/BUILD b/source/extensions/watchdog/abort_action/BUILD
index 21d1a4304a06..96c12f6c3c84 100644
--- a/source/extensions/watchdog/abort_action/BUILD
+++ b/source/extensions/watchdog/abort_action/BUILD
@@ -35,7 +35,6 @@ envoy_cc_extension(
         "//source/common/config:utility_lib",
         "//source/common/protobuf",
         "//source/common/protobuf:message_validator_lib",
-        "//source/extensions/watchdog:well_known_names",
         "@envoy_api//envoy/extensions/watchdog/abort_action/v3alpha:pkg_cc_proto",
     ],
 )
diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index 30296e1b5bad..12cb2d254c16 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -18,9 +18,9 @@ namespace Watchdog {
 namespace AbortAction {
 namespace {
 #ifdef __linux__
-pid_t to_platform_tid(int64_t tid) { return static_cast<pid_t>(tid); }
+pid_t toPlatformTid(int64_t tid) { return static_cast<pid_t>(tid); }
 #elif defined(__APPLE__)
-uint64 to_platform_tid(int64_t tid) { return static_cast<uint64>(tid); }
+uint64 toPlatformTid(int64_t tid) { return static_cast<uint64>(tid); }
 #endif
 } // namespace
 
@@ -48,7 +48,7 @@ void AbortAction::run(
   // Assume POSIX-compatible system and signal to the thread.
   ENVOY_LOG_MISC(error, "AbortAction sending abort signal to thread with tid {}.", raw_tid);
 
-  if (kill(to_platform_tid(raw_tid), SIGABRT) == 0) {
+  if (kill(toPlatformTid(raw_tid), SIGABRT) == 0) {
     // Successfully sent signal, sleep for wait_duration.
     absl::SleepFor(absl::Milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(config_, wait_duration, 0)));
   } else {
@@ -56,7 +56,7 @@ void AbortAction::run(
     ENVOY_LOG_MISC(error, "Failed to send signal to tid {}", raw_tid);
   }
 
-  // Abort from the action since the signaled thread hasn't yet crashed the proccess.
+  // Abort from the action since the signaled thread hasn't yet crashed the process.
   PANIC(
       fmt::format("Failed to kill thread with id {}, aborting from AbortAction instead.", raw_tid));
 #endif
diff --git a/source/extensions/watchdog/abort_action/config.h b/source/extensions/watchdog/abort_action/config.h
index c2c7aef9c73f..3ec2a099861e 100644
--- a/source/extensions/watchdog/abort_action/config.h
+++ b/source/extensions/watchdog/abort_action/config.h
@@ -5,8 +5,6 @@
 
 #include "common/protobuf/protobuf.h"
 
-#include "extensions/watchdog/well_known_names.h"
-
 namespace Envoy {
 namespace Extensions {
 namespace Watchdog {
@@ -14,7 +12,7 @@ namespace AbortAction {
 
 class AbortActionFactory : public Server::Configuration::GuardDogActionFactory {
 public:
-  AbortActionFactory() : name_(WatchdogActionNames::get().AbortAction){};
+  AbortActionFactory() : name_("envoy.watchdog.abort_action") {}
 
   Server::Configuration::GuardDogActionPtr createGuardDogActionFromProto(
       const envoy::config::bootstrap::v3::Watchdog::WatchdogAction& config,
diff --git a/test/extensions/watchdog/abort_action/abort_action_test.cc b/test/extensions/watchdog/abort_action/abort_action_test.cc
index 2ad32ed9aa2c..38c24d1c7266 100644
--- a/test/extensions/watchdog/abort_action/abort_action_test.cc
+++ b/test/extensions/watchdog/abort_action/abort_action_test.cc
@@ -56,7 +56,7 @@ TEST_F(AbortActionTest, ShouldNotAbortIfNoTids) {
   const auto now = api_->timeSource().monotonicTime();
   const std::vector<std::pair<Thread::ThreadId, MonotonicTime>> tid_ltt_pairs = {};
 
-  // Should not signal or panic since there are no tids.
+  // Should not signal or panic since there are no TIDs.
   action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::KILL, tid_ltt_pairs, now);
 }
 
@@ -119,7 +119,7 @@ TEST_F(AbortActionTest, PanicsIfThreadDoesNotDie) {
       std::memset(&saction, 0, sizeof(saction));
       saction.sa_flags = SA_SIGINFO;
       saction.sa_sigaction = &handler;
-      sigaction(SIGABRT, &saction, NULL);
+      sigaction(SIGABRT, &saction, nullptr);
 
       // Signal to test thread that tid has been set.
       {
diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt
index 4647007122f2..2104a9075992 100644
--- a/tools/spelling/spelling_dictionary.txt
+++ b/tools/spelling/spelling_dictionary.txt
@@ -1021,6 +1021,7 @@ setsockopt
 sig
 sigaction
 sigactions
+siginfo
 signalstack
 siloed
 sim

From e2dcb421b3c360835c7d89d441be24cd16cfcfee Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Fri, 28 Aug 2020 21:04:40 +0000
Subject: [PATCH 03/11] minor test fixes.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 source/extensions/watchdog/abort_action/abort_action.cc    | 2 +-
 test/extensions/watchdog/abort_action/abort_action_test.cc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index 12cb2d254c16..cdc58aadf313 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -20,7 +20,7 @@ namespace {
 #ifdef __linux__
 pid_t toPlatformTid(int64_t tid) { return static_cast<pid_t>(tid); }
 #elif defined(__APPLE__)
-uint64 toPlatformTid(int64_t tid) { return static_cast<uint64>(tid); }
+uint64_t toPlatformTid(int64_t tid) { return static_cast<uint64_t>(tid); }
 #endif
 } // namespace
 
diff --git a/test/extensions/watchdog/abort_action/abort_action_test.cc b/test/extensions/watchdog/abort_action/abort_action_test.cc
index 38c24d1c7266..8374c93993d6 100644
--- a/test/extensions/watchdog/abort_action/abort_action_test.cc
+++ b/test/extensions/watchdog/abort_action/abort_action_test.cc
@@ -95,7 +95,7 @@ TEST_F(AbortActionTest, CanKillThread) {
     action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::KILL, tid_ltt_pairs, now);
   };
 
-  EXPECT_DEATH(die_function(), "Caught Aborted");
+  EXPECT_DEATH(die_function(), "");
 }
 
 void handler(int sig, siginfo_t* /*siginfo*/, void* /*context*/) {

From 33a4f5e6ab30aa1c6b22b461f3c07b247c24ac7f Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Fri, 28 Aug 2020 22:24:22 +0000
Subject: [PATCH 04/11] merge conflict fix.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 include/envoy/thread/thread.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h
index 2eee3c743640..6a2cce96eeb0 100644
--- a/include/envoy/thread/thread.h
+++ b/include/envoy/thread/thread.h
@@ -25,7 +25,6 @@ class ThreadId {
 
   int64_t getId() const { return id_; }
   std::string debugString() const { return std::to_string(id_); }
-  int64_t getId() const { return id_; }
   bool isEmpty() const { return *this == ThreadId(); }
   friend bool operator==(ThreadId lhs, ThreadId rhs) { return lhs.id_ == rhs.id_; }
   friend bool operator!=(ThreadId lhs, ThreadId rhs) { return lhs.id_ != rhs.id_; }

From 71f099f09a6e9bab1e039202b2edc304e5f47dd4 Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Mon, 31 Aug 2020 21:50:47 +0000
Subject: [PATCH 05/11] minor fixes

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 .../abort_action/v3alpha/abort_action.proto   |  4 +-
 docs/root/version_history/current.rst         |  2 +-
 .../abort_action/v3alpha/abort_action.proto   |  4 +-
 source/extensions/watchdog/abort_action/BUILD |  2 +-
 .../watchdog/abort_action/abort_action.cc     | 13 +++---
 .../watchdog/abort_action/abort_action.h      |  5 ++-
 .../extensions/watchdog/abort_action/config.h |  6 +--
 .../abort_action/abort_action_test.cc         | 40 ++++---------------
 8 files changed, 27 insertions(+), 49 deletions(-)

diff --git a/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto b/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
index e659444e429d..25e99c9df71f 100644
--- a/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
+++ b/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
@@ -14,12 +14,12 @@ option java_multiple_files = true;
 option (udpa.annotations.file_status).work_in_progress = true;
 option (udpa.annotations.file_status).package_version_status = ACTIVE;
 
-// [#protodoc-title: Watchdog Action that does CPU profiling.]
+// [#protodoc-title: Watchdog Action that sends a SIGABRT to kill the process.]
 // [#extension: envoy.watchdog.abort_action]
 
 // Configuration for the profile watchdog action.
 message AbortActionConfig {
-  // How long to wait for the thread to respond to the abort before killing the
+  // How long to wait for the thread to respond to the SIGABRT before killing the
   // process from this action. This is a blocking action.
   google.protobuf.Duration wait_duration = 1;
 }
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
index 06c813204c33..4bf3030901ca 100644
--- a/docs/root/version_history/current.rst
+++ b/docs/root/version_history/current.rst
@@ -100,7 +100,7 @@ New Features
 * watchdog: support randomizing the watchdog's kill timeout to prevent synchronized kills via a maximium jitter parameter :ref:`max_kill_timeout_jitter<envoy_v3_api_field_config.bootstrap.v3.Watchdog.max_kill_timeout_jitter>`.
 * watchdog: supports an extension point where actions can be registered to fire on watchdog events such as miss, megamiss, kill and multikill. See ref:`watchdog actions<envoy_v3_api_field_config.bootstrap.v3.Watchdog.actions>`.
 * watchdog: watchdog action extension that does cpu profiling. See ref:`Profile Action <envoy_v3_api_file_envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto>`.
-* watchdog: watchdog action extension that signals an abort to a particular thread to kill the process. See ref:`Abort Action <envoy_v3_api_file_envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto>`.
+* watchdog: watchdog action extension that sends SIGABRT to the stuck thread to terminate the process. See ref:`Abort Action <envoy_v3_api_file_envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto>`.
 * xds: added :ref:`extension config discovery<envoy_v3_api_msg_config.core.v3.ExtensionConfigSource>` support for HTTP filters.
 * zlib: added option to use `zlib-ng <https://github.com/zlib-ng/zlib-ng>`_ as zlib library.
 
diff --git a/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
index e659444e429d..25e99c9df71f 100644
--- a/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
+++ b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
@@ -14,12 +14,12 @@ option java_multiple_files = true;
 option (udpa.annotations.file_status).work_in_progress = true;
 option (udpa.annotations.file_status).package_version_status = ACTIVE;
 
-// [#protodoc-title: Watchdog Action that does CPU profiling.]
+// [#protodoc-title: Watchdog Action that sends a SIGABRT to kill the process.]
 // [#extension: envoy.watchdog.abort_action]
 
 // Configuration for the profile watchdog action.
 message AbortActionConfig {
-  // How long to wait for the thread to respond to the abort before killing the
+  // How long to wait for the thread to respond to the SIGABRT before killing the
   // process from this action. This is a blocking action.
   google.protobuf.Duration wait_duration = 1;
 }
diff --git a/source/extensions/watchdog/abort_action/BUILD b/source/extensions/watchdog/abort_action/BUILD
index 96c12f6c3c84..67305bc86e33 100644
--- a/source/extensions/watchdog/abort_action/BUILD
+++ b/source/extensions/watchdog/abort_action/BUILD
@@ -27,7 +27,7 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream_and_upstream",
+    security_posture = "data_plane_agnostic",
     status = "alpha",
     deps = [
         ":abort_action_lib",
diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index cdc58aadf313..58f5ba046062 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -35,18 +35,19 @@ void AbortAction::run(
     MonotonicTime /*now*/) {
 
   if (thread_ltt_pairs.empty()) {
-    ENVOY_LOG_MISC(warn, "AbortAction called without any thread.");
+    ENVOY_LOG_MISC(warn, "Watchdog AbortAction called without any thread.");
     return;
   }
 
-  int64_t raw_tid = thread_ltt_pairs.at(0).first.getId();
+  int64_t raw_tid = thread_ltt_pairs[0].first.getId();
 
 #ifdef WIN32
   // TODO(kbaichoo): add support for this with windows.
-  ENVOY_LOG_MISC(error, "AbortAction is unimplemented for Windows.");
+  ENVOY_LOG_MISC(error, "Watchdog AbortAction is unimplemented for Windows.");
 #else
   // Assume POSIX-compatible system and signal to the thread.
-  ENVOY_LOG_MISC(error, "AbortAction sending abort signal to thread with tid {}.", raw_tid);
+  ENVOY_LOG_MISC(error, "Watchdog AbortAction sending abort signal to thread with tid {}.",
+                 raw_tid);
 
   if (kill(toPlatformTid(raw_tid), SIGABRT) == 0) {
     // Successfully sent signal, sleep for wait_duration.
@@ -57,8 +58,8 @@ void AbortAction::run(
   }
 
   // Abort from the action since the signaled thread hasn't yet crashed the process.
-  PANIC(
-      fmt::format("Failed to kill thread with id {}, aborting from AbortAction instead.", raw_tid));
+  PANIC(fmt::format("Failed to kill thread with id {}, aborting from Watchdog AbortAction instead.",
+                    raw_tid));
 #endif
 }
 
diff --git a/source/extensions/watchdog/abort_action/abort_action.h b/source/extensions/watchdog/abort_action/abort_action.h
index b9e03d71474c..6f006f71c832 100644
--- a/source/extensions/watchdog/abort_action/abort_action.h
+++ b/source/extensions/watchdog/abort_action/abort_action.h
@@ -12,7 +12,10 @@ namespace Watchdog {
 namespace AbortAction {
 
 /**
- * A GuardDogAction that will start signal to a particular thread to abort.
+ * A GuardDogAction that will terminate the process by sending SIGABRT to the
+ * stuck thread. This would allow easier access to the call stack of the stuck
+ * thread since we would run signal handlers on that thread.
+ *
  * This is currently only implemented for systems that support kill to send
  * signals.
  */
diff --git a/source/extensions/watchdog/abort_action/config.h b/source/extensions/watchdog/abort_action/config.h
index 3ec2a099861e..d9f11c562b71 100644
--- a/source/extensions/watchdog/abort_action/config.h
+++ b/source/extensions/watchdog/abort_action/config.h
@@ -12,7 +12,7 @@ namespace AbortAction {
 
 class AbortActionFactory : public Server::Configuration::GuardDogActionFactory {
 public:
-  AbortActionFactory() : name_("envoy.watchdog.abort_action") {}
+  AbortActionFactory() = default;
 
   Server::Configuration::GuardDogActionPtr createGuardDogActionFromProto(
       const envoy::config::bootstrap::v3::Watchdog::WatchdogAction& config,
@@ -22,11 +22,9 @@ class AbortActionFactory : public Server::Configuration::GuardDogActionFactory {
     return std::make_unique<AbortActionConfig>();
   }
 
-  std::string name() const override { return name_; }
+  std::string name() const override { return "envoy.watchdog.abort_action"; }
 
-private:
   using AbortActionConfig = envoy::extensions::watchdog::abort_action::v3alpha::AbortActionConfig;
-  const std::string name_;
 };
 
 } // namespace AbortAction
diff --git a/test/extensions/watchdog/abort_action/abort_action_test.cc b/test/extensions/watchdog/abort_action/abort_action_test.cc
index 8374c93993d6..214564ba898a 100644
--- a/test/extensions/watchdog/abort_action/abort_action_test.cc
+++ b/test/extensions/watchdog/abort_action/abort_action_test.cc
@@ -13,7 +13,7 @@
 
 #include "test/test_common/utility.h"
 
-#include "absl/synchronization/mutex.h"
+#include "absl/synchronization/notification.h"
 #include "gtest/gtest.h"
 
 namespace Envoy {
@@ -30,21 +30,13 @@ class AbortActionTest : public testing::Test {
       : api_(Api::createApiForTest()), dispatcher_(api_->allocateDispatcher("test")),
         context_({*api_, *dispatcher_}) {}
 
-  void waitForOutstandingNotify() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_) {
-    mutex_.Await(absl::Condition(
-        +[](int* outstanding_notifies) -> bool { return *outstanding_notifies > 0; },
-        &outstanding_notifies_));
-    outstanding_notifies_ -= 1;
-  }
-
   Api::ApiPtr api_;
   Event::DispatcherPtr dispatcher_;
   Server::Configuration::GuardDogActionFactoryContext context_;
   std::unique_ptr<Server::Configuration::GuardDogAction> action_;
 
-  // Used to synchronize with the dispatch thread
-  absl::Mutex mutex_;
-  int outstanding_notifies_ ABSL_GUARDED_BY(mutex_) = 0;
+  // Used to synchronize with the main thread
+  absl::Notification child_ready_;
 };
 
 TEST_F(AbortActionTest, ShouldNotAbortIfNoTids) {
@@ -73,20 +65,12 @@ TEST_F(AbortActionTest, CanKillThread) {
     Thread::ThreadPtr thread = api_->threadFactory().createThread([this, &tid]() -> void {
       tid = api_->threadFactory().currentThreadId();
 
-      // Signal to test thread that tid has been set.
-      {
-        absl::MutexLock lock(&mutex_);
-        outstanding_notifies_ += 1;
-      }
+      child_ready_.Notify();
 
       dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit);
     });
 
-    // Wait for child thread to notify of its tid
-    {
-      absl::MutexLock lock(&mutex_);
-      waitForOutstandingNotify();
-    }
+    child_ready_.WaitForNotification();
 
     // Create vector with child tid and run the action.
     const auto now = api_->timeSource().monotonicTime();
@@ -121,20 +105,12 @@ TEST_F(AbortActionTest, PanicsIfThreadDoesNotDie) {
       saction.sa_sigaction = &handler;
       sigaction(SIGABRT, &saction, nullptr);
 
-      // Signal to test thread that tid has been set.
-      {
-        absl::MutexLock lock(&mutex_);
-        outstanding_notifies_ += 1;
-      }
+      child_ready_.Notify();
 
       dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit);
     });
 
-    // Wait for child thread to notify of its tid
-    {
-      absl::MutexLock lock(&mutex_);
-      waitForOutstandingNotify();
-    }
+    child_ready_.WaitForNotification();
 
     // Create vector with child tid and run the action.
     const auto now = api_->timeSource().monotonicTime();
@@ -143,7 +119,7 @@ TEST_F(AbortActionTest, PanicsIfThreadDoesNotDie) {
     action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::KILL, tid_ltt_pairs, now);
   };
 
-  EXPECT_DEATH(die_function(), "aborting from AbortAction instead");
+  EXPECT_DEATH(die_function(), "aborting from Watchdog AbortAction instead");
 }
 
 #endif

From 847aed0f8951e23b8a0999abf242127ba887c778 Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Thu, 10 Sep 2020 02:57:41 +0000
Subject: [PATCH 06/11] minor fixes.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 .../abort_action/v3alpha/abort_action.proto   |  9 +++++++-
 .../abort_action/v3alpha/abort_action.proto   |  9 +++++++-
 include/envoy/server/guarddog_config.h        | 11 +++++----
 .../watchdog/abort_action/abort_action.cc     |  6 ++---
 .../watchdog/abort_action/abort_action.h      |  9 +++-----
 .../watchdog/profile_action/profile_action.cc |  8 +++----
 .../watchdog/profile_action/profile_action.h  |  4 ++--
 source/server/guarddog_impl.cc                | 23 ++++++++++---------
 source/server/guarddog_impl.h                 |  8 +++----
 test/server/guarddog_impl_test.cc             | 13 ++++++-----
 tools/spelling/spelling_dictionary.txt        |  1 +
 11 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto b/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
index 25e99c9df71f..7d793be82012 100644
--- a/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
+++ b/api/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
@@ -17,7 +17,14 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#protodoc-title: Watchdog Action that sends a SIGABRT to kill the process.]
 // [#extension: envoy.watchdog.abort_action]
 
-// Configuration for the profile watchdog action.
+// A GuardDogAction that will terminate the process by sending SIGABRT to the
+// stuck thread. This would allow easier access to the call stack of the stuck
+// thread since we would run signal handlers on that thread. This would be
+// more useful than the default watchdog kill behaviors since those PANIC
+// from the watchdog's thread.
+
+// This is currently only implemented for systems that support kill to send
+// signals.
 message AbortActionConfig {
   // How long to wait for the thread to respond to the SIGABRT before killing the
   // process from this action. This is a blocking action.
diff --git a/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
index 25e99c9df71f..7d793be82012 100644
--- a/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
+++ b/generated_api_shadow/envoy/extensions/watchdog/abort_action/v3alpha/abort_action.proto
@@ -17,7 +17,14 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#protodoc-title: Watchdog Action that sends a SIGABRT to kill the process.]
 // [#extension: envoy.watchdog.abort_action]
 
-// Configuration for the profile watchdog action.
+// A GuardDogAction that will terminate the process by sending SIGABRT to the
+// stuck thread. This would allow easier access to the call stack of the stuck
+// thread since we would run signal handlers on that thread. This would be
+// more useful than the default watchdog kill behaviors since those PANIC
+// from the watchdog's thread.
+
+// This is currently only implemented for systems that support kill to send
+// signals.
 message AbortActionConfig {
   // How long to wait for the thread to respond to the SIGABRT before killing the
   // process from this action. This is a blocking action.
diff --git a/include/envoy/server/guarddog_config.h b/include/envoy/server/guarddog_config.h
index 4c51778a7eb3..907a9186f759 100644
--- a/include/envoy/server/guarddog_config.h
+++ b/include/envoy/server/guarddog_config.h
@@ -27,13 +27,14 @@ class GuardDogAction {
   /**
    * Callback function for when the GuardDog observes an event.
    * @param event the event the GuardDog observes.
-   * @param thread_ltt_pairs pairs of the relevant thread to the event, and the
-   *  last time touched (LTT) of those threads with their watchdog.
+   * @param thread_last_checkin_pairs pair of the relevant thread to the event, and the
+   *  last check in time of those threads with their watchdog.
    * @param now the current time.
    */
-  virtual void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event,
-                   const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
-                   MonotonicTime now) PURE;
+  virtual void
+  run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event,
+      const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs,
+      MonotonicTime now) PURE;
 };
 
 using GuardDogActionPtr = std::unique_ptr<GuardDogAction>;
diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index 58f5ba046062..1f73a57d0ebf 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -31,15 +31,15 @@ AbortAction::AbortAction(
 
 void AbortAction::run(
     envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent /*event*/,
-    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
+    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs,
     MonotonicTime /*now*/) {
 
-  if (thread_ltt_pairs.empty()) {
+  if (thread_last_checkin_pairs.empty()) {
     ENVOY_LOG_MISC(warn, "Watchdog AbortAction called without any thread.");
     return;
   }
 
-  int64_t raw_tid = thread_ltt_pairs[0].first.getId();
+  int64_t raw_tid = thread_last_checkin_pairs[0].first.getId();
 
 #ifdef WIN32
   // TODO(kbaichoo): add support for this with windows.
diff --git a/source/extensions/watchdog/abort_action/abort_action.h b/source/extensions/watchdog/abort_action/abort_action.h
index 6f006f71c832..90b64393080b 100644
--- a/source/extensions/watchdog/abort_action/abort_action.h
+++ b/source/extensions/watchdog/abort_action/abort_action.h
@@ -13,11 +13,8 @@ namespace AbortAction {
 
 /**
  * A GuardDogAction that will terminate the process by sending SIGABRT to the
- * stuck thread. This would allow easier access to the call stack of the stuck
- * thread since we would run signal handlers on that thread.
- *
- * This is currently only implemented for systems that support kill to send
- * signals.
+ * stuck thread. This is currently only implemented for systems that
+ * support kill to send signals.
  */
 class AbortAction : public Server::Configuration::GuardDogAction {
 public:
@@ -25,7 +22,7 @@ class AbortAction : public Server::Configuration::GuardDogAction {
               Server::Configuration::GuardDogActionFactoryContext& context);
 
   void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event,
-           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
+           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs,
            MonotonicTime now) override;
 
 private:
diff --git a/source/extensions/watchdog/profile_action/profile_action.cc b/source/extensions/watchdog/profile_action/profile_action.cc
index 6d63c3cf23f9..1e7f8509b48a 100644
--- a/source/extensions/watchdog/profile_action/profile_action.cc
+++ b/source/extensions/watchdog/profile_action/profile_action.cc
@@ -51,14 +51,14 @@ ProfileAction::ProfileAction(
 
 void ProfileAction::run(
     envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent /*event*/,
-    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
+    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs,
     MonotonicTime /*now*/) {
   if (running_profile_) {
     return;
   }
 
   // Check if there's a tid that justifies profiling
-  auto trigger_tid = getTidTriggeringProfile(thread_ltt_pairs);
+  auto trigger_tid = getTidTriggeringProfile(thread_last_checkin_pairs);
   if (!trigger_tid.has_value()) {
     ENVOY_LOG_MISC(warn, "Profile Action: None of the provided tids justify profiling");
     return;
@@ -92,10 +92,10 @@ void ProfileAction::run(
 
 // Helper to determine if we have a valid tid to start profiling.
 absl::optional<Thread::ThreadId> ProfileAction::getTidTriggeringProfile(
-    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs) {
+    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs) {
 
   // Find a TID not over the max_profiles threshold
-  for (const auto& [tid, ltt] : thread_ltt_pairs) {
+  for (const auto& [tid, last_checkin] : thread_last_checkin_pairs) {
     if (tid_to_profile_count_[tid] < max_profiles_per_tid_) {
       return tid;
     }
diff --git a/source/extensions/watchdog/profile_action/profile_action.h b/source/extensions/watchdog/profile_action/profile_action.h
index e91a2cfd3615..cbf1128b4827 100644
--- a/source/extensions/watchdog/profile_action/profile_action.h
+++ b/source/extensions/watchdog/profile_action/profile_action.h
@@ -22,13 +22,13 @@ class ProfileAction : public Server::Configuration::GuardDogAction {
                 Server::Configuration::GuardDogActionFactoryContext& context);
 
   void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event,
-           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
+           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs,
            MonotonicTime now) override;
 
 private:
   // Helper to determine if we should run the profiler.
   absl::optional<Thread::ThreadId> getTidTriggeringProfile(
-      const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs);
+      const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs);
 
   const std::string path_;
   const std::chrono::milliseconds duration_;
diff --git a/source/server/guarddog_impl.cc b/source/server/guarddog_impl.cc
index cce74e493d9e..e10cfde63b01 100644
--- a/source/server/guarddog_impl.cc
+++ b/source/server/guarddog_impl.cc
@@ -101,10 +101,10 @@ void GuardDogImpl::step() {
                  static_cast<size_t>(ceil(multi_kill_fraction_ * watched_dogs_.size())));
 
     for (auto& watched_dog : watched_dogs_) {
-      const auto ltt = watched_dog->dog_->lastTouchTime();
+      const auto last_checkin = watched_dog->dog_->lastTouchTime();
       const auto tid = watched_dog->dog_->threadId();
-      const auto delta = now - ltt;
-      if (watched_dog->last_alert_time_ && watched_dog->last_alert_time_.value() < ltt) {
+      const auto delta = now - last_checkin;
+      if (watched_dog->last_alert_time_ && watched_dog->last_alert_time_.value() < last_checkin) {
         watched_dog->miss_alerted_ = false;
         watched_dog->megamiss_alerted_ = false;
       }
@@ -112,28 +112,28 @@ void GuardDogImpl::step() {
         if (!watched_dog->miss_alerted_) {
           watchdog_miss_counter_.inc();
           watched_dog->miss_counter_.inc();
-          watched_dog->last_alert_time_ = ltt;
+          watched_dog->last_alert_time_ = last_checkin;
           watched_dog->miss_alerted_ = true;
-          miss_threads.emplace_back(tid, ltt);
+          miss_threads.emplace_back(tid, last_checkin);
         }
       }
       if (delta > megamiss_timeout_) {
         if (!watched_dog->megamiss_alerted_) {
           watchdog_megamiss_counter_.inc();
           watched_dog->megamiss_counter_.inc();
-          watched_dog->last_alert_time_ = ltt;
+          watched_dog->last_alert_time_ = last_checkin;
           watched_dog->megamiss_alerted_ = true;
-          mega_miss_threads.emplace_back(tid, ltt);
+          mega_miss_threads.emplace_back(tid, last_checkin);
         }
       }
       if (killEnabled() && delta > kill_timeout_) {
-        invokeGuardDogActions(WatchDogAction::KILL, {{tid, ltt}}, now);
+        invokeGuardDogActions(WatchDogAction::KILL, {{tid, last_checkin}}, now);
 
         PANIC(fmt::format("GuardDog: one thread ({}) stuck for more than watchdog_kill_timeout",
                           watched_dog->dog_->threadId().debugString()));
       }
       if (multikillEnabled() && delta > multi_kill_timeout_) {
-        multi_kill_threads.emplace_back(tid, ltt);
+        multi_kill_threads.emplace_back(tid, last_checkin);
 
         if (multi_kill_threads.size() >= required_for_multi_kill) {
           invokeGuardDogActions(WatchDogAction::MULTIKILL, multi_kill_threads, now);
@@ -216,11 +216,12 @@ void GuardDogImpl::stop() {
 
 void GuardDogImpl::invokeGuardDogActions(
     WatchDogAction::WatchdogEvent event,
-    std::vector<std::pair<Thread::ThreadId, MonotonicTime>> thread_ltt_pairs, MonotonicTime now) {
+    std::vector<std::pair<Thread::ThreadId, MonotonicTime>> thread_last_checkin_pairs,
+    MonotonicTime now) {
   const auto& registered_actions = events_to_actions_.find(event);
   if (registered_actions != events_to_actions_.end()) {
     for (auto& action : registered_actions->second) {
-      action->run(event, thread_ltt_pairs, now);
+      action->run(event, thread_last_checkin_pairs, now);
     }
   }
 }
diff --git a/source/server/guarddog_impl.h b/source/server/guarddog_impl.h
index b854215bbbd9..5653c3eb6be1 100644
--- a/source/server/guarddog_impl.h
+++ b/source/server/guarddog_impl.h
@@ -104,10 +104,10 @@ class GuardDogImpl : public GuardDog {
 
   using WatchDogAction = envoy::config::bootstrap::v3::Watchdog::WatchdogAction;
   // Helper function to invoke all the GuardDogActions registered for an Event.
-  void
-  invokeGuardDogActions(WatchDogAction::WatchdogEvent event,
-                        std::vector<std::pair<Thread::ThreadId, MonotonicTime>> thread_ltt_pairs,
-                        MonotonicTime now);
+  void invokeGuardDogActions(
+      WatchDogAction::WatchdogEvent event,
+      std::vector<std::pair<Thread::ThreadId, MonotonicTime>> thread_last_checkin_pairs,
+      MonotonicTime now);
 
   struct WatchedDog {
     WatchedDog(Stats::Scope& stats_scope, const std::string& thread_name,
diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc
index 49fe6f73f3f3..a66fd0a00812 100644
--- a/test/server/guarddog_impl_test.cc
+++ b/test/server/guarddog_impl_test.cc
@@ -444,15 +444,15 @@ class RecordGuardDogAction : public Configuration::GuardDogAction {
   RecordGuardDogAction(std::vector<std::string>& events) : events_(events) {}
 
   void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event,
-           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs,
+           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_last_checkin_pairs,
            MonotonicTime /*now*/) override {
     std::string event_string =
         envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent_Name(event);
     absl::StrAppend(&event_string, " : ");
     std::vector<std::string> output_string_parts;
-    output_string_parts.reserve(thread_ltt_pairs.size());
+    output_string_parts.reserve(thread_last_checkin_pairs.size());
 
-    for (const auto& thread_ltt_pair : thread_ltt_pairs) {
+    for (const auto& thread_ltt_pair : thread_last_checkin_pairs) {
       output_string_parts.push_back(thread_ltt_pair.first.debugString());
     }
 
@@ -469,9 +469,10 @@ class AssertGuardDogAction : public Configuration::GuardDogAction {
 public:
   AssertGuardDogAction() = default;
 
-  void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent /*event*/,
-           const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& /*thread_ltt_pairs*/,
-           MonotonicTime /*now*/) override {
+  void
+  run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent /*event*/,
+      const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& /*thread_last_checkin_pairs*/,
+      MonotonicTime /*now*/) override {
     RELEASE_ASSERT(false, "ASSERT_GUARDDOG_ACTION");
   }
 };
diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt
index 2104a9075992..06294899545e 100644
--- a/tools/spelling/spelling_dictionary.txt
+++ b/tools/spelling/spelling_dictionary.txt
@@ -468,6 +468,7 @@ canonicalizing
 cardinality
 casted
 charset
+checkin
 checksum
 chrono
 chroot

From 371866f01f6263d6ca643a0f1c39ff6df6274d7b Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Mon, 14 Sep 2020 14:41:59 +0000
Subject: [PATCH 07/11] Added abort_action to WINDOWS_SKIP_TARGETS.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 bazel/repositories.bzl                           |  1 +
 .../watchdog/abort_action/abort_action.cc        | 16 +++++-----------
 .../watchdog/abort_action/abort_action_test.cc   |  3 ---
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl
index f66dc9f180ed..5ded74dddffe 100644
--- a/bazel/repositories.bzl
+++ b/bazel/repositories.bzl
@@ -12,6 +12,7 @@ WINDOWS_SKIP_TARGETS = [
     "envoy.tracers.lightstep",
     "envoy.tracers.datadog",
     "envoy.tracers.opencensus",
+    "envoy.watchdog.abort_action",
 ]
 
 # Make all contents of an external repository accessible under a filegroup.  Used for external HTTP
diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index 1f73a57d0ebf..66f4f5cfcefc 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -1,16 +1,15 @@
 #include "extensions/watchdog/abort_action/abort_action.h"
 
-#include "common/common/assert.h"
-#include "common/common/fmt.h"
-
-#ifndef WIN32
 #include <sys/types.h>
+
 #include <csignal>
-#endif
 
+#include "envoy/thread/thread.h"
+
+#include "common/common/assert.h"
+#include "common/common/fmt.h"
 #include "common/common/logger.h"
 #include "common/protobuf/utility.h"
-#include "envoy/thread/thread.h"
 
 namespace Envoy {
 namespace Extensions {
@@ -41,10 +40,6 @@ void AbortAction::run(
 
   int64_t raw_tid = thread_last_checkin_pairs[0].first.getId();
 
-#ifdef WIN32
-  // TODO(kbaichoo): add support for this with windows.
-  ENVOY_LOG_MISC(error, "Watchdog AbortAction is unimplemented for Windows.");
-#else
   // Assume POSIX-compatible system and signal to the thread.
   ENVOY_LOG_MISC(error, "Watchdog AbortAction sending abort signal to thread with tid {}.",
                  raw_tid);
@@ -60,7 +55,6 @@ void AbortAction::run(
   // Abort from the action since the signaled thread hasn't yet crashed the process.
   PANIC(fmt::format("Failed to kill thread with id {}, aborting from Watchdog AbortAction instead.",
                     raw_tid));
-#endif
 }
 
 } // namespace AbortAction
diff --git a/test/extensions/watchdog/abort_action/abort_action_test.cc b/test/extensions/watchdog/abort_action/abort_action_test.cc
index 214564ba898a..0d3157ffec12 100644
--- a/test/extensions/watchdog/abort_action/abort_action_test.cc
+++ b/test/extensions/watchdog/abort_action/abort_action_test.cc
@@ -53,7 +53,6 @@ TEST_F(AbortActionTest, ShouldNotAbortIfNoTids) {
 }
 
 // insufficient signal support on Windows.
-#ifndef WIN32
 TEST_F(AbortActionTest, CanKillThread) {
   AbortActionConfig config;
   config.mutable_wait_duration()->set_seconds(1);
@@ -122,8 +121,6 @@ TEST_F(AbortActionTest, PanicsIfThreadDoesNotDie) {
   EXPECT_DEATH(die_function(), "aborting from Watchdog AbortAction instead");
 }
 
-#endif
-
 } // namespace
 } // namespace AbortAction
 } // namespace Watchdog

From 80f2dc0c13eba132a6dddf737a490742dcb87d91 Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Tue, 15 Sep 2020 13:27:32 +0000
Subject: [PATCH 08/11] added comment about panic. added skip on windows tags.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 source/extensions/watchdog/abort_action/abort_action.cc | 2 ++
 test/extensions/watchdog/abort_action/BUILD             | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index 66f4f5cfcefc..09533d32aed9 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -53,6 +53,8 @@ void AbortAction::run(
   }
 
   // Abort from the action since the signaled thread hasn't yet crashed the process.
+  // panicing in the action gives flexibility since it doesn't depend on
+  // external code to kill the process if the signal fails.
   PANIC(fmt::format("Failed to kill thread with id {}, aborting from Watchdog AbortAction instead.",
                     raw_tid));
 }
diff --git a/test/extensions/watchdog/abort_action/BUILD b/test/extensions/watchdog/abort_action/BUILD
index 1a7acd173ac7..30d2c13b2999 100644
--- a/test/extensions/watchdog/abort_action/BUILD
+++ b/test/extensions/watchdog/abort_action/BUILD
@@ -18,6 +18,7 @@ envoy_extension_cc_test(
     external_deps = [
         "abseil_synchronization",
     ],
+    tags = ["skip_on_windows"],
     deps = [
         "//include/envoy/common:time_interface",
         "//include/envoy/registry",
@@ -34,6 +35,7 @@ envoy_extension_cc_test(
     name = "config_test",
     srcs = ["config_test.cc"],
     extension_name = "envoy.watchdog.abort_action",
+    tags = ["skip_on_windows"],
     deps = [
         "//include/envoy/registry",
         "//include/envoy/server:guarddog_config_interface",

From 56b3f820e9fac1a64ede5883300b66ea8600f625 Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Tue, 15 Sep 2020 21:43:44 +0000
Subject: [PATCH 09/11] Added LCOV threshold that will get covered by tested
 that aren't DEATH tests.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 test/per_file_coverage.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh
index 99339d151f9a..4d7a41c2f720 100755
--- a/test/per_file_coverage.sh
+++ b/test/per_file_coverage.sh
@@ -68,6 +68,7 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/extensions/transport_sockets/tls/private_key:76.9"
 "source/extensions/watchdog:84.9"
 "source/extensions/watchdog/profile_action:84.9"
+"source/extensions/watchdog/abort_action:45.0"
 "source/server:94.6"
 "source/server/config_validation:76.8"
 "source/server/admin:95.5"

From 5fb84a905f58feb1903d05c50221a6804615688f Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Tue, 15 Sep 2020 21:59:31 +0000
Subject: [PATCH 10/11] Added additional death test comments.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 source/extensions/watchdog/abort_action/abort_action.cc | 2 ++
 test/per_file_coverage.sh                               | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index 09533d32aed9..57c9da136b36 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -38,6 +38,8 @@ void AbortAction::run(
     return;
   }
 
+  // The following liens of code won't be considered coverage by code coverage
+  // tools since they would run in DEATH tests.
   int64_t raw_tid = thread_last_checkin_pairs[0].first.getId();
 
   // Assume POSIX-compatible system and signal to the thread.
diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh
index 4d7a41c2f720..0bde072fd442 100755
--- a/test/per_file_coverage.sh
+++ b/test/per_file_coverage.sh
@@ -66,9 +66,9 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/extensions/transport_sockets/tls:94.2"
 "source/extensions/transport_sockets/tls/ocsp:95.3"
 "source/extensions/transport_sockets/tls/private_key:76.9"
-"source/extensions/watchdog:84.9"
+"source/extensions/watchdog:70.8" # Death tests within extensions
 "source/extensions/watchdog/profile_action:84.9"
-"source/extensions/watchdog/abort_action:45.0"
+"source/extensions/watchdog/abort_action:45.0" # Death tests don't report LCOV
 "source/server:94.6"
 "source/server/config_validation:76.8"
 "source/server/admin:95.5"

From 947bd06a0e9349f7553b918f50d3d3cdf46f9af6 Mon Sep 17 00:00:00 2001
From: Kevin Baichoo <kbaichoo@google.com>
Date: Wed, 16 Sep 2020 00:38:09 +0000
Subject: [PATCH 11/11] Adjusted coverage threshold since comments and empty
 new lines affect line coverage percent.

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
---
 source/extensions/watchdog/abort_action/abort_action.cc | 2 +-
 test/per_file_coverage.sh                               | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/source/extensions/watchdog/abort_action/abort_action.cc b/source/extensions/watchdog/abort_action/abort_action.cc
index 57c9da136b36..3a4c3ade615e 100644
--- a/source/extensions/watchdog/abort_action/abort_action.cc
+++ b/source/extensions/watchdog/abort_action/abort_action.cc
@@ -38,7 +38,7 @@ void AbortAction::run(
     return;
   }
 
-  // The following liens of code won't be considered coverage by code coverage
+  // The following lines of code won't be considered covered by code coverage
   // tools since they would run in DEATH tests.
   int64_t raw_tid = thread_last_checkin_pairs[0].first.getId();
 
diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh
index 0bde072fd442..70d00a616eed 100755
--- a/test/per_file_coverage.sh
+++ b/test/per_file_coverage.sh
@@ -66,9 +66,9 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/extensions/transport_sockets/tls:94.2"
 "source/extensions/transport_sockets/tls/ocsp:95.3"
 "source/extensions/transport_sockets/tls/private_key:76.9"
-"source/extensions/watchdog:70.8" # Death tests within extensions
+"source/extensions/watchdog:69.6" # Death tests within extensions
 "source/extensions/watchdog/profile_action:84.9"
-"source/extensions/watchdog/abort_action:45.0" # Death tests don't report LCOV
+"source/extensions/watchdog/abort_action:42.9" # Death tests don't report LCOV
 "source/server:94.6"
 "source/server/config_validation:76.8"
 "source/server/admin:95.5"