diff --git a/NOTICE b/NOTICE index 4b9ffb79ab1..949f852011e 100644 --- a/NOTICE +++ b/NOTICE @@ -35,7 +35,7 @@ Copyright (C) 2013 GoDaddy Operating Company, LLC ~~~ -lib/cppapi developed by LinkedIn +include/tscpp/api, src/tscpp/api developed by LinkedIn Copyright (c) 2013 LinkedIn ~~~ diff --git a/build/plugins.mk b/build/plugins.mk index 4556fc22e2d..a0f0d32690c 100644 --- a/build/plugins.mk +++ b/build/plugins.mk @@ -25,10 +25,6 @@ TS_PLUGIN_LD_FLAGS = \ -export-symbols-regex '^(TSRemapInit|TSRemapDone|TSRemapDoRemap|TSRemapNewInstance|TSRemapDeleteInstance|TSRemapOSResponse|TSPluginInit|TSRemapPreConfigReload|TSRemapPostConfigReload)$$' TS_PLUGIN_CPPFLAGS = \ - -I$(abs_top_builddir)/proxy/api \ - -I$(abs_top_srcdir)/proxy/api \ - -I$(abs_top_srcdir)/include/cppapi/include \ - -I$(abs_top_builddir)/lib/cppapi/include \ -I$(abs_top_srcdir)/include \ -I$(abs_top_srcdir)/lib diff --git a/doc/developer-guide/api/functions/TSHttpHookAdd.en.rst b/doc/developer-guide/api/functions/TSHttpHookAdd.en.rst index 9536a3f88d1..b2c80b1ec0e 100644 --- a/doc/developer-guide/api/functions/TSHttpHookAdd.en.rst +++ b/doc/developer-guide/api/functions/TSHttpHookAdd.en.rst @@ -44,11 +44,13 @@ function for callback amounts to adding the function to a hook. You can register your plugin to be called back for every single transaction, or for specific transactions only. -HTTP :term:`transaction` hooks are set on a global basis using the function -:func:`TSHttpHookAdd`. This means that the continuation specified -as the parameter to :func:`TSHttpHookAdd` is called for every -transaction. :func:`TSHttpHookAdd` must only be called from -:func:`TSPluginInit` or :func:`TSRemapInit`. +HTTP :term:`transaction` and :term:`session` hooks are set on a +global basis using the function :func:`TSHttpHookAdd`. This means +that the continuation specified as the parameter to :func:`TSHttpHookAdd` +is called for every transaction. :func:`TSHttpHookAdd` must only be called from +:func:`TSPluginInit` or :func:`TSRemapInit`. Continuations set on a +global hook will run before any continuations set on the session/transaction +hook with the same hook ID. :func:`TSHttpSsnHookAdd` adds :arg:`contp` to the end of the list of HTTP :term:`session` hooks specified by :arg:`id`. @@ -56,14 +58,20 @@ This means that :arg:`contp` is called back for every transaction within the session, at the point specified by the hook ID. Since :arg:`contp` is added to a session, it is not possible to call :func:`TSHttpSsnHookAdd` from the plugin initialization routine; -the plugin needs a handle to an HTTP session. +the plugin needs a handle to an HTTP session. Continuations set on a +session hook will run before any continuations set on the transaction +hook with the same hook ID. This fucnction can be called from handler +functions of continuations on a global per-session hook, including for +the session hook with the same ID. :func:`TSHttpTxnHookAdd` adds :arg:`contp` to the end of the list of HTTP transaction hooks specified by :arg:`id`. Since :arg:`contp` is added to a transaction, it is not possible to call :func:`TSHttpTxnHookAdd` from the plugin initialization routine but only when the plugin has a handle to an -HTTP transaction. +HTTP transaction. This fucnction can be called from handler +functions of continuations on a global or session per-transaction +hook, including the for transaction hook with the same ID. A single continuation can be attached to multiple hooks at the same time. It is good practice to conserve resources by reusing hooks in this way diff --git a/doc/developer-guide/api/functions/TSMutexLock.en.rst b/doc/developer-guide/api/functions/TSMutexLock.en.rst index 11558f2dbae..3cfc0bdbc37 100644 --- a/doc/developer-guide/api/functions/TSMutexLock.en.rst +++ b/doc/developer-guide/api/functions/TSMutexLock.en.rst @@ -32,3 +32,6 @@ Synopsis Description =========== + +Locks the mutex (recursively). Should only be called from a continuation +handler function or a :type:`TSThread` function. diff --git a/doc/developer-guide/api/functions/TSMutexUnlock.en.rst b/doc/developer-guide/api/functions/TSMutexUnlock.en.rst index 37a96a99f23..6f34bc85ba3 100644 --- a/doc/developer-guide/api/functions/TSMutexUnlock.en.rst +++ b/doc/developer-guide/api/functions/TSMutexUnlock.en.rst @@ -32,3 +32,9 @@ Synopsis Description =========== + +Decrements the recursive lock count. If the count thus becomes zero, unlocks +the mutex. This should only be called within the continuation handler function or +:type:`TSThread` function that locked the mutex. Can also be called within +the continuation handler for the continuation's mutex. (This is normally done +before the continution handler destroys the continuation running it.) diff --git a/doc/developer-guide/plugins/continuations/writing-handler-functions.en.rst b/doc/developer-guide/plugins/continuations/writing-handler-functions.en.rst index f081ff7079c..b609cd35eda 100644 --- a/doc/developer-guide/plugins/continuations/writing-handler-functions.en.rst +++ b/doc/developer-guide/plugins/continuations/writing-handler-functions.en.rst @@ -140,3 +140,10 @@ The continuation functions are listed below: - :func:`TSContSchedule` - :func:`TSContScheduleOnPool` - :func:`TSContScheduleOnThread` + +When a handler function blocks, it blocks the event thread running it. This blocks all the continuations (internal ones +along with those of plugins) in the event thread's queue. This may increase the worst-case latency for HTTP request +processing. If there is enough blocking, this could increase CPU idle time, which may reduce proxy throughput. The +Au test **polite_hook_wait** illustrates a method for using dynamic threading to do a blocking call without blocking +any handler function. But the overhead of this method may cancel out the performance improvement, if blocking times +are short. diff --git a/plugins/experimental/fastcgi/src/Readme b/plugins/experimental/fastcgi/src/Readme index 24cb8c6085e..20eb093529a 100644 --- a/plugins/experimental/fastcgi/src/Readme +++ b/plugins/experimental/fastcgi/src/Readme @@ -32,7 +32,6 @@ tsxs -o ats_fastcgi.so \ -c ats_fastcgi.cc \ -L "${ats_dir}lib/" \ -I "${ats_dir}lib" \ - -I "${ats_dir}lib/cppapi/include/" \ -c ats_fcgi_client.cc \ -latscppapi diff --git a/tests/Makefile.am b/tests/Makefile.am index 391078815e5..f3aa84a0aa1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -33,6 +33,7 @@ AM_LDFLAGS += -rpath $(abs_builddir) include gold_tests/bigobj/Makefile.inc include gold_tests/continuations/plugins/Makefile.inc include gold_tests/chunked_encoding/Makefile.inc +include gold_tests/pluginTest/polite_hook_wait/Makefile.inc include gold_tests/pluginTest/tsapi/Makefile.inc include gold_tests/timeout/Makefile.inc include gold_tests/tls/Makefile.inc diff --git a/tests/gold_tests/pluginTest/polite_hook_wait/Makefile.inc b/tests/gold_tests/pluginTest/polite_hook_wait/Makefile.inc new file mode 100644 index 00000000000..ff87816ffcf --- /dev/null +++ b/tests/gold_tests/pluginTest/polite_hook_wait/Makefile.inc @@ -0,0 +1,18 @@ +# 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. + +noinst_LTLIBRARIES += gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.la +gold_tests_pluginTest_polite_hook_wait_polite_hook_wait_la_SOURCES = gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.cc diff --git a/tests/gold_tests/pluginTest/polite_hook_wait/curl.gold b/tests/gold_tests/pluginTest/polite_hook_wait/curl.gold new file mode 100644 index 00000000000..be03905cec8 --- /dev/null +++ b/tests/gold_tests/pluginTest/polite_hook_wait/curl.gold @@ -0,0 +1,2 @@ +> GET / HTTP/1.1 +< HTTP/1.1 403 Forbidden diff --git a/tests/gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.cc b/tests/gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.cc new file mode 100644 index 00000000000..bba59407c85 --- /dev/null +++ b/tests/gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.cc @@ -0,0 +1,272 @@ +/* + * 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. + */ + +#include +#include +#include + +#include +#include + +using atscppapi::TSContUniqPtr; +using atscppapi::TSThreadUniqPtr; + +/* +Test handling a blocking call (in a spawned thread) on a transaction hook without blocking the thread executing the hooks. + +It is dependent on continuations hooked globally on a transaction hook running before continations hooked for just the +one transaction. It is dependent on the ability of a global continuation on a txn hook to add a per-txn continuation on +the same hook. +*/ + +#define PINAME "polite_hook_wait" + +namespace +{ +char PIName[] = PINAME; + +auto dbg_ctl{TSDbgCtlCreate(PIName)}; + +enum Test_step +{ + BEGIN, + GLOBAL_CONT_READ_HDRS, + THREAD, + TXN_CONT_READ_HDRS, + END +}; + +char const *step_cstr(int test_step) +{ + char const *result{"BAD TEST STEP"}; + + switch (test_step) + { + case BEGIN: + result = "BEGIN"; + break; + + case GLOBAL_CONT_READ_HDRS: + result = "GLOBAL_CONT_READ_HDRS"; + break; + + case THREAD: + result = "THREAD"; + break; + + case TXN_CONT_READ_HDRS: + result = "TXN_CONT_READ_HDRS"; + break; + + default: + break; + } + + return result; +} + +int txn_count{0}; + +void next_step(int curr) +{ + static std::atomic test_step{BEGIN}; + + TSReleaseAssert(test_step.load(std::memory_order_relaxed) == curr); + + if (BEGIN == curr) { + ++txn_count; + + TSReleaseAssert(txn_count <= 2); + } + + ++curr; + if (END == curr) { + curr = BEGIN; + } + + TSDbg(dbg_ctl, "Entering test step %s", step_cstr(curr)); + + test_step.store(curr, std::memory_order_relaxed); +} + +atscppapi::TxnAuxMgrData mgr_data; + +class Blocking_action +{ +public: + + static void init(); + +private: + + ~Blocking_action() + { + // This should either not block, or only block very briefly. + // + TSThreadWait(_checker.get()); + + TSDbg(dbg_ctl, "In ~Blocking_action()"); + } + + Blocking_action() = default; + + static int _global_cont_func(TSCont, TSEvent event, void *eventData); + static int _txn_cont_func(TSCont, TSEvent event, void *eventData); + static void * _thread_func(void *vba); + + TSContUniqPtr _txn_hook_cont{TSContCreate(_txn_cont_func, TSMutexCreate())}; + std::atomic _cont_mutex_locked{false}; + + TSThreadUniqPtr _checker{TSThreadCreate(&_thread_func, this)}; + + bool txn_valid{false}; + + friend class atscppapi::TxnAuxDataMgr; +}; + +using AuxDataMgr = atscppapi::TxnAuxDataMgr; + +void +Blocking_action::init() +{ + static TSContUniqPtr global{TSContCreate(_global_cont_func, nullptr)}; + + TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, global.get()); + TSHttpHookAdd(TS_HTTP_SEND_RESPONSE_HDR_HOOK, global.get()); +} + +int Blocking_action::_global_cont_func(TSCont, TSEvent event, void *eventData) +{ + TSDbg(dbg_ctl, "entering _global_cont_func()"); + + TSReleaseAssert(eventData != nullptr); + + TSHttpTxn txn{static_cast(eventData)}; + + switch (event) + { + case TS_EVENT_HTTP_READ_REQUEST_HDR: { + next_step(BEGIN); + + Blocking_action &ba = AuxDataMgr::data(txn); + + if (!ba._checker.get()) { + TSError(PINAME ": failed to create thread"); + TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE); + return 0; + } + + TSHttpTxnHookAdd(txn, TS_HTTP_READ_REQUEST_HDR_HOOK, ba._txn_hook_cont.get()); + + while (!ba._cont_mutex_locked.load(std::memory_order_acquire)) { + std::this_thread::yield(); + } + } + break; + + case TS_EVENT_HTTP_SEND_RESPONSE_HDR: + next_step(TXN_CONT_READ_HDRS); + + if (!AuxDataMgr::data(txn).txn_valid) { + static const char msg[] = "authorization denied\n"; + + TSHttpTxnErrorBodySet(txn, TSstrdup(msg), sizeof(msg) - 1, TSstrdup("text/plain")); + } + + break; + + default: + TSReleaseAssert(false); + break; + } + + TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE); + + return 0; +} + +void * Blocking_action::_thread_func(void *vba) +{ + next_step(GLOBAL_CONT_READ_HDRS); + + auto ba = static_cast(vba); + + TSMutexLock(TSContMutexGet(ba->_txn_hook_cont.get())); // This will never block. + ba->_cont_mutex_locked.store(true, std::memory_order_release); + + // This is a stand-in for some blocking call to validate the HTTP request in some way. + // + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + // Pass "validation" for first transaction, fail it for second. + // + if (1 == txn_count) { + ba->txn_valid = true; + } + + // Let per-txn continuation run. + // + TSMutexUnlock(TSContMutexGet(ba->_txn_hook_cont.get())); + + return nullptr; +} + +int Blocking_action::_txn_cont_func(TSCont, TSEvent event, void *eventData) +{ + next_step(THREAD); + + TSReleaseAssert(eventData != nullptr); + TSReleaseAssert(TS_EVENT_HTTP_READ_REQUEST_HDR == event); + + TSHttpTxn txn{static_cast(eventData)}; + + Blocking_action &ba = AuxDataMgr::data(txn); + + if (!ba.txn_valid) { + TSHttpTxnStatusSet(txn, TS_HTTP_STATUS_FORBIDDEN); + } + + TSHttpTxnReenable(txn, ba.txn_valid ? TS_EVENT_HTTP_CONTINUE : TS_EVENT_HTTP_ERROR); + + return 0; +} + +} // end anonymous namespace + +void +TSPluginInit(int n_arg, char const *arg[]) +{ + TSDbg(dbg_ctl, "initializing plugin"); + + TSPluginRegistrationInfo info; + + info.plugin_name = const_cast(PIName); + info.vendor_name = const_cast("apache"); + info.support_email = const_cast("edge@yahooinc.com"); + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError(PINAME ": failure calling TSPluginRegister."); + return; + } else { + TSDbg(dbg_ctl, "Plugin registration succeeded."); + } + + AuxDataMgr::init(PIName); + + Blocking_action::init(); +} diff --git a/tests/gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.test.py b/tests/gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.test.py new file mode 100644 index 00000000000..8e865e763c0 --- /dev/null +++ b/tests/gold_tests/pluginTest/polite_hook_wait/polite_hook_wait.test.py @@ -0,0 +1,73 @@ +# 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 spawning a thread in a transaction hook continuation, and getting a result from it, without blocking event task. +''' + +plugin_name = "polite_hook_wait" + +server = Test.MakeOriginServer("server") + +request_header = { + "headers": "GET / 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": "112233"} +server.addResponse("sessionlog.json", request_header, response_header) + + +# Disable the cache to make sure each request is forwarded to the origin +# server. +ts = Test.MakeATSProcess("ts", enable_cache=False, block_for_debug=False) + +ts.Disk.records_config.update({ + 'proxy.config.proxy_name': 'Poxy_Proxy', # This will be the server name. + 'proxy.config.url_remap.remap_required': 1, + 'proxy.config.diags.debug.enabled': 3, + 'proxy.config.diags.debug.tags': f'{plugin_name}', +}) + +rp = os.path.join(Test.Variables.AtsBuildGoldTestsDir, 'pluginTest', 'polite_hook_wait', '.libs', f'{plugin_name}.so') +ts.Setup.Copy(rp, ts.Env['PROXY_CONFIG_PLUGIN_PLUGIN_DIR']) + +ts.Disk.plugin_config.AddLine( + f"{plugin_name}.so" +) + +ts.Disk.remap_config.AddLine( + "map http://myhost.test http://127.0.0.1:{0}".format(server.Variables.Port) +) + +tr = Test.AddTestRun() +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = ( + 'curl --verbose --ipv4 --header "Host:myhost.test" http://localhost:{}/ 2>curl.txt'.format(ts.Variables.port) +) +tr.Processes.Default.ReturnCode = 0 + +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + 'curl --verbose --ipv4 --header "Host:myhost.test" http://localhost:{}/ 2>curl.txt'.format(ts.Variables.port) +) +tr.Processes.Default.ReturnCode = 0 + +tr = Test.AddTestRun() +tr.Processes.Default.Command = "grep -F HTTP/ curl.txt" +tr.Processes.Default.Streams.stdout = "curl.gold" +tr.Processes.Default.ReturnCode = 0