From 39f443f36bdc3ff6242863cce9a24589c37a294c Mon Sep 17 00:00:00 2001
From: Oknet <xuchao@gmail.com>
Date: Wed, 10 Oct 2018 13:50:10 +0800
Subject: [PATCH] Assert if operations on keep_alive_queue and active_queue are
 not thread-safe

The `vc->add_to_keep_alive_queue()` is called by:
  - Http1ClientSession::release()
  - Http2ClientSession::cleanup_streams()
  - Http2ClientSession::release_stream()

And the `NetHandler::remove_from_active_queue()` is called by
`vc->add_to_keep_alive_queue()`.

The NetHandler::keep_alive_queue and NetHandler::active_queue are
internal queue of NetHandler. We must have acquired the NetHandler's
lock before doing anything with them.

When reenable a HttpSM from plugin, the HttpSM would be run into ethread
that different from client_vc lives because TSHttpSMCallback is
scheduled by `eventProcessor.schedule_imm()`.

(cherry picked from commit c929ed6fdee34a67529b55f9fe1c47d449ee854f)
---
 iocore/net/UnixNet.cc            |  6 ++++++
 iocore/net/UnixNetVConnection.cc | 31 +++++++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/iocore/net/UnixNet.cc b/iocore/net/UnixNet.cc
index 06da4146762..a0cdc50f2ba 100644
--- a/iocore/net/UnixNet.cc
+++ b/iocore/net/UnixNet.cc
@@ -689,6 +689,7 @@ void
 NetHandler::add_to_keep_alive_queue(UnixNetVConnection *vc)
 {
   Debug("net_queue", "NetVC: %p", vc);
+  ink_assert(mutex->thread_holding == this_ethread());
 
   if (keep_alive_queue.in(vc)) {
     // already in the keep-alive queue, move the head
@@ -708,6 +709,8 @@ void
 NetHandler::remove_from_keep_alive_queue(UnixNetVConnection *vc)
 {
   Debug("net_queue", "NetVC: %p", vc);
+  ink_assert(mutex->thread_holding == this_ethread());
+
   if (keep_alive_queue.in(vc)) {
     keep_alive_queue.remove(vc);
     --keep_alive_queue_size;
@@ -720,6 +723,7 @@ NetHandler::add_to_active_queue(UnixNetVConnection *vc)
   Debug("net_queue", "NetVC: %p", vc);
   Debug("net_queue", "max_connections_per_thread_in: %d active_queue_size: %d keep_alive_queue_size: %d",
         max_connections_per_thread_in, active_queue_size, keep_alive_queue_size);
+  ink_assert(mutex->thread_holding == this_ethread());
 
   // if active queue is over size then close inactive connections
   if (manage_active_queue() == false) {
@@ -744,6 +748,8 @@ void
 NetHandler::remove_from_active_queue(UnixNetVConnection *vc)
 {
   Debug("net_queue", "NetVC: %p", vc);
+  ink_assert(mutex->thread_holding == this_ethread());
+
   if (active_queue.in(vc)) {
     active_queue.remove(vc);
     --active_queue_size;
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index c2303fa22c6..c8953af3002 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -1543,23 +1543,46 @@ UnixNetVConnection::migrateToCurrentThread(Continuation *cont, EThread *t)
 void
 UnixNetVConnection::add_to_keep_alive_queue()
 {
-  nh->add_to_keep_alive_queue(this);
+  MUTEX_TRY_LOCK(lock, nh->mutex, this_ethread());
+  if (lock.is_locked()) {
+    nh->add_to_keep_alive_queue(this);
+  } else {
+    ink_release_assert(!"BUG: It must have acquired the NetHandler's lock before doing anything on keep_alive_queue.");
+  }
 }
 
 void
 UnixNetVConnection::remove_from_keep_alive_queue()
 {
-  nh->remove_from_keep_alive_queue(this);
+  MUTEX_TRY_LOCK(lock, nh->mutex, this_ethread());
+  if (lock.is_locked()) {
+    nh->remove_from_keep_alive_queue(this);
+  } else {
+    ink_release_assert(!"BUG: It must have acquired the NetHandler's lock before doing anything on keep_alive_queue.");
+  }
 }
 
 bool
 UnixNetVConnection::add_to_active_queue()
 {
-  return nh->add_to_active_queue(this);
+  bool result = false;
+
+  MUTEX_TRY_LOCK(lock, nh->mutex, this_ethread());
+  if (lock.is_locked()) {
+    result = nh->add_to_active_queue(this);
+  } else {
+    ink_release_assert(!"BUG: It must have acquired the NetHandler's lock before doing anything on active_queue.");
+  }
+  return result;
 }
 
 void
 UnixNetVConnection::remove_from_active_queue()
 {
-  nh->remove_from_active_queue(this);
+  MUTEX_TRY_LOCK(lock, nh->mutex, this_ethread());
+  if (lock.is_locked()) {
+    nh->remove_from_active_queue(this);
+  } else {
+    ink_release_assert(!"BUG: It must have acquired the NetHandler's lock before doing anything on active_queue.");
+  }
 }