This is an automated email from the ASF dual-hosted git repository.
shinrich 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 ae094ab Avoid the auto-reschedule with dispatchEvent
ae094ab is described below
commit ae094ab660eb67fc9ec019c086de8a4b07c914f4
Author: Susan Hinrichs <shinr...@oath.com>
AuthorDate: Fri Nov 30 16:21:44 2018 +0000
Avoid the auto-reschedule with dispatchEvent
---
.../api/functions/TSContCall.en.rst | 25 +++++--------
iocore/eventsystem/Continuation.cc | 42 ----------------------
iocore/eventsystem/I_Continuation.h | 14 --------
iocore/eventsystem/Makefile.am | 1 -
iocore/net/UnixNetVConnection.cc | 11 ++++--
proxy/http/HttpConfig.cc | 6 +++-
proxy/http/HttpSM.cc | 10 +++---
proxy/http/HttpTunnel.cc | 4 +--
src/traffic_server/InkAPI.cc | 22 ++++++++++--
9 files changed, 50 insertions(+), 85 deletions(-)
diff --git a/doc/developer-guide/api/functions/TSContCall.en.rst
b/doc/developer-guide/api/functions/TSContCall.en.rst
index 9da6a31..5453bb3 100644
--- a/doc/developer-guide/api/functions/TSContCall.en.rst
+++ b/doc/developer-guide/api/functions/TSContCall.en.rst
@@ -44,24 +44,17 @@ As a result :func:`TSContCall` will effectively do::
return CallbackHandler(contp, event, edata);
-:func:`TSContCall` will check :arg:`contp` for a mutex. If there is a mutex an
attempt will be made
-to lock that mutex. If either there is no mutex, or the mutex lock was
acquired, the handler will be
-called directly. Otherwise there is a mutex and it was not successfully
locked. In that case an
-event will be scheduled to dispatch as soon as possible, but not in the
current call stack. The
-nature of event dispatch means the event will not be dispatched until the
mutext can be locked. In
-all cases the handler in :arg:`contp` will be called with the same arguments.
:func:`TSContCall`
-will return 0 if a mutex was present but the lock was not acquired. Otherwise
it will return the
+If there is a mutex associated with :arg:`contp`, :func:`TSComtCall` assumes
that mutex is held already.
+:func:`TSContCall` will directly call the handler associated with the
continuation. It will return the
value returned by the handler in :arg:`contp`.
-If the scheduling behavior of :func:`TSContCall` isn't appropriate, either
:arg:`contp` must not
-have a mutex, or the plugin must acquire the lock on the mutex for
:arg:`contp` before calling
-:func:`TSContCall`. See :func:`TSContMutexGet` and :func:`TSMutexLockTry` for
mechanisms for doing
-the latter. This is what :func:`TSContCall` does internally, and should be
done by the plugin only
-if a different approach for waiting for the lock is needed. The most common
case is the code called
-by :func:`TSContCall` must complete before further code is executed at the
call site. An alternative
-approach to handling the locking directly would be to split the call site in
to two continuations,
-one of which is signalled (possibly via :func:`TSContCall`) from the original
:func:`TSContCall`
-target.
+If :arg:`contp` has a mutex, the plugin must acquire the lock on the mutex for
:arg:`contp` before calling
+:func:`TSContCall`. See :func:`TSContMutexGet` and :func:`TSMutexLockTry` for
mechanisms for doing this.
+
+The most common case is the code called by :func:`TSContCall` must complete
before further code is executed
+at the call site. An alternative approach to handling the locking directly
would be to split the call site
+into two continuations, one of which is signalled (possibly via
:func:`TSContCall`) from the original
+:func:`TSContCall` target.
Note mutexes returned by :func:`TSMutexCreate` are recursive mutexes,
therefore if the lock is
already held on the thread of execution acquiring the lock again is very fast.
Mutexes are also
diff --git a/iocore/eventsystem/Continuation.cc
b/iocore/eventsystem/Continuation.cc
deleted file mode 100644
index c693632..0000000
--- a/iocore/eventsystem/Continuation.cc
+++ /dev/null
@@ -1,42 +0,0 @@
-/** @file
-
- Contination.cc
-
- @section license License
-
- 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 "I_EventSystem.h"
-#include "I_Continuation.h"
-
-int
-Continuation::dispatchEvent(int event, void *data)
-{
- if (mutex) {
- EThread *t = this_ethread();
- MUTEX_TRY_LOCK(lock, this->mutex, t);
- if (!lock.is_locked()) {
- t->schedule_imm(this, event, data);
- return 0;
- } else {
- return (this->*handler)(event, data);
- }
- } else {
- return (this->*handler)(event, data);
- }
-}
diff --git a/iocore/eventsystem/I_Continuation.h
b/iocore/eventsystem/I_Continuation.h
index bd73f71..140a11d 100644
--- a/iocore/eventsystem/I_Continuation.h
+++ b/iocore/eventsystem/I_Continuation.h
@@ -165,20 +165,6 @@ public:
return (this->*handler)(event, data);
}
- /**
- Receives the event code and data for an Event.
-
- It will attempt to get the lock for the continuation, and reschedule
- the event if the lock cannot be obtained. If the lock can be obtained
- dispatchEvent acts like handleEvent.
-
- @param event Event code to be passed at callback (Processor specific).
- @param data General purpose data related to the event code (Processor
specific).
- @return State machine and processor specific return code.
-
- */
- int dispatchEvent(int event = CONTINUATION_EVENT_NONE, void *data = nullptr);
-
protected:
/**
Constructor of the Continuation object. It should not be used
diff --git a/iocore/eventsystem/Makefile.am b/iocore/eventsystem/Makefile.am
index d481f40..6f44a16 100644
--- a/iocore/eventsystem/Makefile.am
+++ b/iocore/eventsystem/Makefile.am
@@ -59,7 +59,6 @@ libinkevent_a_SOURCES = \
P_UnixSocketManager.h \
P_VConnection.h \
P_VIO.h \
- Continuation.cc \
Processor.cc \
ProtectedQueue.cc \
ProxyAllocator.cc \
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index ca5e341..2e9edfc 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -1087,8 +1087,15 @@ UnixNetVConnection::acceptEvent(int event, Event *e)
if (active_timeout_in) {
UnixNetVConnection::set_active_timeout(active_timeout_in);
}
-
- action_.continuation->dispatchEvent(NET_EVENT_ACCEPT, this);
+ if (action_.continuation->mutex != nullptr) {
+ MUTEX_TRY_LOCK(lock3, action_.continuation->mutex, t);
+ if (!lock3.is_locked()) {
+ ink_release_assert(0);
+ }
+ action_.continuation->handleEvent(NET_EVENT_ACCEPT, this);
+ } else {
+ action_.continuation->handleEvent(NET_EVENT_ACCEPT, this);
+ }
return EVENT_DONE;
}
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 13b1125..887fe5e 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1221,7 +1221,11 @@ HttpConfig::startup()
OutboundConnTrack::config_init(&c.outbound_conntrack,
&c.oride.outbound_conntrack);
- http_config_cont->dispatchEvent(EVENT_NONE, nullptr);
+ MUTEX_TRY_LOCK(lock, http_config_cont->mutex, this_ethread());
+ if (!lock.is_locked()) {
+ ink_release_assert(0);
+ }
+ http_config_cont->handleEvent(EVENT_NONE, nullptr);
return;
}
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 48ccfc6..514e9f9 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -555,7 +555,7 @@ HttpSM::setup_client_read_request_header()
ua_entry->read_vio = ua_txn->do_io_read(this, INT64_MAX,
ua_buffer_reader->mbuf);
// The header may already be in the buffer if this
// a request from a keep-alive connection
- dispatchEvent(VC_EVENT_READ_READY, ua_entry->read_vio);
+ handleEvent(VC_EVENT_READ_READY, ua_entry->read_vio);
}
void
@@ -888,7 +888,7 @@ HttpSM::state_watch_for_client_abort(int event, void *data)
"[%" PRId64 "] [watch_for_client_abort] "
"forwarding event %s to tunnel",
sm_id, HttpDebugNames::get_event_name(event));
- tunnel.dispatchEvent(event, c->write_vio);
+ tunnel.handleEvent(event, c->write_vio);
return 0;
} else {
tunnel.kill_tunnel();
@@ -4015,7 +4015,7 @@ HttpSM::do_remap_request(bool run_inline)
if (!ret) {
SMDebug("url_rewrite", "Could not find a valid remapping entry for this
request [%" PRId64 "]", sm_id);
if (!run_inline) {
- dispatchEvent(EVENT_REMAP_COMPLETE, nullptr);
+ handleEvent(EVENT_REMAP_COMPLETE, nullptr);
}
return;
}
@@ -5425,13 +5425,13 @@ HttpSM::handle_server_setup_error(int event, void *data)
ua_producer->alive = false;
ua_producer->handler_state = HTTP_SM_POST_SERVER_FAIL;
- tunnel.dispatchEvent(VC_EVENT_ERROR, c->write_vio);
+ tunnel.handleEvent(VC_EVENT_ERROR, c->write_vio);
return;
}
} else {
// c could be null here as well
if (c != nullptr) {
- tunnel.dispatchEvent(event, c->write_vio);
+ tunnel.handleEvent(event, c->write_vio);
return;
}
}
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index 86ca78b..84634c3 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -776,7 +776,7 @@ HttpTunnel::tunnel_run(HttpTunnelProducer *p_arg)
// back to say we are done
if (!is_tunnel_alive()) {
active = false;
- sm->dispatchEvent(HTTP_TUNNEL_EVENT_DONE, this);
+ sm->handleEvent(HTTP_TUNNEL_EVENT_DONE, this);
}
}
@@ -1640,7 +1640,7 @@ HttpTunnel::main_handler(int event, void *data)
if (reentrancy_count == 1) {
reentrancy_count = 0;
active = false;
- sm->dispatchEvent(HTTP_TUNNEL_EVENT_DONE, this);
+ sm->handleEvent(HTTP_TUNNEL_EVENT_DONE, this);
return EVENT_DONE;
} else {
call_sm = true;
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index fc18b72..810435a 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -1295,7 +1295,16 @@ APIHook::invoke(int event, void *edata)
ink_assert(!"not reached");
}
}
- return m_cont->dispatchEvent(event, edata);
+ if (m_cont->mutex != nullptr) {
+ MUTEX_TRY_LOCK(lock, m_cont->mutex, this_ethread());
+ if (!lock.is_locked()) {
+ // If we cannot get the lock, the caller needs to restructure to handle
rescheduling
+ ink_release_assert(0);
+ }
+ return m_cont->handleEvent(event, edata);
+ } else {
+ return m_cont->handleEvent(event, edata);
+ }
}
APIHook *
@@ -4510,7 +4519,16 @@ int
TSContCall(TSCont contp, TSEvent event, void *edata)
{
Continuation *c = (Continuation *)contp;
- return c->dispatchEvent((int)event, edata);
+ if (c->mutex != nullptr) {
+ MUTEX_TRY_LOCK(lock, c->mutex, this_ethread());
+ if (!lock.is_locked()) {
+ // If we cannot get the lock, the caller needs to restructure to handle
rescheduling
+ ink_release_assert(0);
+ }
+ return c->handleEvent((int)event, edata);
+ } else {
+ return c->handleEvent((int)event, edata);
+ }
}
TSMutex
