This is an automated email from the ASF dual-hosted git repository. oknet pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push: new c929ed6 Assert if operations on keep_alive_queue and active_queue are not thread-safe c929ed6 is described below commit c929ed6fdee34a67529b55f9fe1c47d449ee854f Author: Oknet <xuc...@gmail.com> AuthorDate: Wed Oct 10 13:50:10 2018 +0800 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()`. --- 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 eb95841..76a10a4 100644 --- a/iocore/net/UnixNet.cc +++ b/iocore/net/UnixNet.cc @@ -673,6 +673,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 @@ -692,6 +693,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; @@ -704,6 +707,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) { @@ -728,6 +732,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 2e9edfc..635c98f 100644 --- a/iocore/net/UnixNetVConnection.cc +++ b/iocore/net/UnixNetVConnection.cc @@ -1451,25 +1451,48 @@ 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."); + } } int