You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2020/12/03 19:36:27 UTC
[trafficserver] 04/07: Get appropriate locks on SSN_START hook
delays (#7295)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit c0ae43ccf9f67098795c024c20be5ee66662a26c
Author: Susan Hinrichs <sh...@yahoo-inc.com>
AuthorDate: Mon Nov 2 12:46:25 2020 -0600
Get appropriate locks on SSN_START hook delays (#7295)
(cherry picked from commit 1765c9f2d6ade367773342983db6973015d70f42)
---
proxy/http/HttpSessionAccept.cc | 2 +
proxy/http2/Http2SessionAccept.cc | 3 ++
src/traffic_server/InkAPI.cc | 36 ++++++++++---
.../pluginTest/test_hooks/ssn_delay.gold | 8 +++
.../test_hooks/ssn_start_delay_hook.test.py | 60 ++++++++++++++++++++++
tests/tools/plugins/hook_add_plugin.cc | 35 +++++++++----
6 files changed, 127 insertions(+), 17 deletions(-)
diff --git a/proxy/http/HttpSessionAccept.cc b/proxy/http/HttpSessionAccept.cc
index e7fd20f..ea9d075 100644
--- a/proxy/http/HttpSessionAccept.cc
+++ b/proxy/http/HttpSessionAccept.cc
@@ -54,6 +54,8 @@ HttpSessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferReade
new_session->accept_options = static_cast<Options *>(this);
new_session->acl = std::move(acl);
+ // Pin session to current ET_NET thread
+ new_session->setThreadAffinity(this_ethread());
new_session->new_connection(netvc, iobuf, reader);
new_session->trans.upstream_outbound_options = *new_session->accept_options;
diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
index 7f68e64..f0226fd 100644
--- a/proxy/http2/Http2SessionAccept.cc
+++ b/proxy/http2/Http2SessionAccept.cc
@@ -56,6 +56,9 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
Http2ClientSession *new_session = THREAD_ALLOC_INIT(http2ClientSessionAllocator, this_ethread());
new_session->acl = std::move(session_acl);
new_session->accept_options = &options;
+
+ // Pin session to current ET_NET thread
+ new_session->setThreadAffinity(this_ethread());
new_session->new_connection(netvc, iobuf, reader);
return true;
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index e02106b..4b2d9af 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -4895,7 +4895,7 @@ TSHttpSsnServerVConnGet(TSHttpSsn ssnp)
class TSHttpSsnCallback : public Continuation
{
public:
- TSHttpSsnCallback(ProxySession *cs, TSEvent event) : Continuation(cs->mutex), m_cs(cs), m_event(event)
+ TSHttpSsnCallback(ProxySession *cs, Ptr<ProxyMutex> m, TSEvent event) : Continuation(m), m_cs(cs), m_event(event)
{
SET_HANDLER(&TSHttpSsnCallback::event_handler);
}
@@ -4903,8 +4903,18 @@ public:
int
event_handler(int, void *)
{
- m_cs->handleEvent((int)m_event, nullptr);
- delete this;
+ // The current continuation is associated with the nethandler mutex.
+ // We need to hold the nethandler mutex because the later Session logic may
+ // activate the nethandler add_to_queue logic
+ // Need to make sure we have the ProxySession mutex as well.
+ EThread *eth = this_ethread();
+ MUTEX_TRY_LOCK(trylock, m_cs->mutex, eth);
+ if (!trylock.is_locked()) {
+ eth->schedule_imm(this);
+ } else {
+ m_cs->handleEvent((int)m_event, nullptr);
+ delete this;
+ }
return 0;
}
@@ -4923,13 +4933,25 @@ TSHttpSsnReenable(TSHttpSsn ssnp, TSEvent event)
// If this function is being executed on a thread created by the API
// which is DEDICATED, the continuation needs to be called back on a
- // REGULAR thread.
- if (eth->tt != REGULAR) {
- eventProcessor.schedule_imm(new TSHttpSsnCallback(cs, event), ET_NET);
+ // REGULAR thread. Specially an ET_NET thread
+ if (!eth->is_event_type(ET_NET)) {
+ EThread *affinity_thread = cs->getThreadAffinity();
+ if (affinity_thread && affinity_thread->is_event_type(ET_NET)) {
+ NetHandler *nh = get_NetHandler(affinity_thread);
+ affinity_thread->schedule_imm(new TSHttpSsnCallback(cs, nh->mutex, event), ET_NET);
+ } else {
+ eventProcessor.schedule_imm(new TSHttpSsnCallback(cs, cs->mutex, event), ET_NET);
+ }
} else {
MUTEX_TRY_LOCK(trylock, cs->mutex, eth);
if (!trylock.is_locked()) {
- eventProcessor.schedule_imm(new TSHttpSsnCallback(cs, event), ET_NET);
+ EThread *affinity_thread = cs->getThreadAffinity();
+ if (affinity_thread && affinity_thread->is_event_type(ET_NET)) {
+ NetHandler *nh = get_NetHandler(affinity_thread);
+ affinity_thread->schedule_imm(new TSHttpSsnCallback(cs, nh->mutex, event), ET_NET);
+ } else {
+ eventProcessor.schedule_imm(new TSHttpSsnCallback(cs, cs->mutex, event), ET_NET);
+ }
} else {
cs->handleEvent((int)event, nullptr);
}
diff --git a/tests/gold_tests/pluginTest/test_hooks/ssn_delay.gold b/tests/gold_tests/pluginTest/test_hooks/ssn_delay.gold
new file mode 100644
index 0000000..0510f3f
--- /dev/null
+++ b/tests/gold_tests/pluginTest/test_hooks/ssn_delay.gold
@@ -0,0 +1,8 @@
+`` DIAG: (test) -- globalHandler :: TS_EVENT_HTTP_SSN_START
+`` DIAG: (test) New session, cont is ``
+`` DIAG: (test) -- sessionHandler :: TS_EVENT_TIMEOUT
+`` DIAG: (test) -- sessionHandler :: TS_EVENT_HTTP_PRE_REMAP
+`` DIAG: (test) -- transactionHandler :: TS_EVENT_HTTP_PRE_REMAP
+`` DIAG: (test) -- transactionHandler :: TS_EVENT_HTTP_TXN_CLOSE
+`` DIAG: (test) -- sessionHandler :: TS_EVENT_HTTP_SSN_CLOSE
+``
diff --git a/tests/gold_tests/pluginTest/test_hooks/ssn_start_delay_hook.test.py b/tests/gold_tests/pluginTest/test_hooks/ssn_start_delay_hook.test.py
new file mode 100644
index 0000000..738630f
--- /dev/null
+++ b/tests/gold_tests/pluginTest/test_hooks/ssn_start_delay_hook.test.py
@@ -0,0 +1,60 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+
+
+Test.Summary = '''
+Test adding hooks, and rescheduling the ssn start hook from a non-net thread
+'''
+
+Test.ContinueOnFail = True
+
+server = Test.MakeOriginServer("server")
+
+request_header = {
+ "headers": "GET /argh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+server.addResponse("sessionlog.json", request_header, response_header)
+
+ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=False)
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.tags': 'test',
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.http.cache.http': 0,
+ 'proxy.config.url_remap.remap_required': 0,
+})
+
+Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'hook_add_plugin.so'), ts, '-delay')
+
+ts.Disk.remap_config.AddLine(
+ "map http://one http://127.0.0.1:{0}".format(server.Variables.Port)
+)
+
+tr = Test.AddTestRun()
+# Probe server port to check if ready.
+tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port))
+# Probe TS cleartext port to check if ready (probing TLS port causes spurious VCONN hook triggers).
+tr.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.port))
+#
+tr.Processes.Default.Command = (
+ 'curl --verbose --ipv4 --header "Host: one" http://localhost:{0}/argh'.format(ts.Variables.port)
+)
+tr.Processes.Default.ReturnCode = 0
+
+# Look at the debug output from the plugin
+ts.Streams.All = "ssn_delay.gold"
diff --git a/tests/tools/plugins/hook_add_plugin.cc b/tests/tools/plugins/hook_add_plugin.cc
index f8fcfe4..87bc859 100644
--- a/tests/tools/plugins/hook_add_plugin.cc
+++ b/tests/tools/plugins/hook_add_plugin.cc
@@ -21,9 +21,13 @@
limitations under the License.
*/
#include <ts/ts.h>
+#include <string.h>
#define PLUGIN_TAG "test"
+// Number of seconds to reschedule to a task thread and delay
+int DelayStart = 0;
+
int
transactionHandler(TSCont continuation, TSEvent event, void *d)
{
@@ -79,6 +83,12 @@ sessionHandler(TSCont continuation, TSEvent event, void *d)
return 0;
} break;
+ case TS_EVENT_TIMEOUT: { // The schedule case, reenable the session continuation
+ TSDebug(PLUGIN_TAG, " -- sessionHandler :: TS_EVENT_TIMEOUT");
+ TSHttpSsn session = static_cast<TSHttpSsn>(TSContDataGet(continuation));
+ TSHttpSsnReenable(session, TS_EVENT_HTTP_CONTINUE);
+ return 0;
+ }
default:
TSAssert(!"Unexpected event");
break;
@@ -91,10 +101,8 @@ sessionHandler(TSCont continuation, TSEvent event, void *d)
int
globalHandler(TSCont continuation, TSEvent event, void *data)
{
- TSHttpSsn session = static_cast<TSHttpSsn>(data);
-
- switch (event) {
- case TS_EVENT_HTTP_SSN_START: {
+ if (event == TS_EVENT_HTTP_SSN_START) {
+ TSHttpSsn session = static_cast<TSHttpSsn>(data);
TSDebug(PLUGIN_TAG, " -- globalHandler :: TS_EVENT_HTTP_SSN_START");
TSCont cont = TSContCreate(sessionHandler, TSMutexCreate());
@@ -102,14 +110,15 @@ globalHandler(TSCont continuation, TSEvent event, void *data)
TSHttpSsnHookAdd(session, TS_HTTP_SSN_CLOSE_HOOK, cont);
TSDebug(PLUGIN_TAG, "New session, cont is %p", cont);
- } break;
- default:
- return 0;
+ if (DelayStart == 0) {
+ TSHttpSsnReenable(session, TS_EVENT_HTTP_CONTINUE);
+ } else {
+ TSContDataSet(cont, session);
+ TSContScheduleOnPool(cont, 500, TS_THREAD_POOL_TASK);
+ }
}
- TSHttpSsnReenable(session, TS_EVENT_HTTP_CONTINUE);
-
return 0;
}
@@ -134,7 +143,13 @@ TSPluginInit(int argc, const char **argv)
return;
}
- TSCont continuation = TSContCreate(globalHandler, nullptr);
+ if (argc >= 2) {
+ TSDebug(PLUGIN_TAG, "Argument %s", argv[1]);
+ if (strcmp(argv[1], "-delay") == 0) {
+ DelayStart = 1;
+ }
+ }
+ TSCont continuation = TSContCreate(globalHandler, TSMutexCreate());
TSHttpHookAdd(TS_HTTP_SSN_START_HOOK, continuation);
}