You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2018/08/01 17:12:07 UTC
[trafficserver] branch master updated: Ensure that continuation
lock is held before calling handler.
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 15fe1ae Ensure that continuation lock is held before calling handler.
15fe1ae is described below
commit 15fe1ae42917be156dece87dfa3c5f983d9a3c75
Author: Susan Hinrichs <sh...@oath.com>
AuthorDate: Thu Jul 26 21:56:03 2018 +0000
Ensure that continuation lock is held before calling handler.
---
iocore/eventsystem/Continuation.cc | 51 +++++++++++++++++++++++++++++++++++++
iocore/eventsystem/I_Continuation.h | 22 +++++++++++-----
iocore/eventsystem/Makefile.am | 1 +
iocore/eventsystem/UnixEThread.cc | 1 +
iocore/net/UnixNetAccept.cc | 2 +-
iocore/net/UnixNetVConnection.cc | 2 +-
proxy/http/HttpConfig.cc | 2 +-
proxy/http/HttpSM.cc | 10 ++++----
proxy/http/HttpTunnel.cc | 4 +--
src/traffic_server/InkAPI.cc | 4 +--
10 files changed, 81 insertions(+), 18 deletions(-)
diff --git a/iocore/eventsystem/Continuation.cc b/iocore/eventsystem/Continuation.cc
new file mode 100644
index 0000000..26de73b
--- /dev/null
+++ b/iocore/eventsystem/Continuation.cc
@@ -0,0 +1,51 @@
+/** @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"
+#include "I_EThread.h"
+
+int
+Continuation::handleEvent(int event, void *data)
+{
+ // If there is a lock, we must be holding it on entry
+ ink_release_assert(!mutex || mutex->thread_holding == this_ethread());
+ return (this->*handler)(event, data);
+}
+
+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 a14ba1c..8d662d6 100644
--- a/iocore/eventsystem/I_Continuation.h
+++ b/iocore/eventsystem/I_Continuation.h
@@ -147,18 +147,28 @@ public:
This function receives the event code and data for an event and
forwards them to the current continuation handler. The processor
calling back the continuation is responsible for acquiring its
- lock.
+ lock. If the lock is present and not held, this method will assert.
@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
- handleEvent(int event = CONTINUATION_EVENT_NONE, void *data = nullptr)
- {
- return (this->*handler)(event, data);
- }
+ int handleEvent(int event = CONTINUATION_EVENT_NONE, void *data = nullptr);
+
+ /**
+ 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:
/**
diff --git a/iocore/eventsystem/Makefile.am b/iocore/eventsystem/Makefile.am
index 5e4d95e..f416598 100644
--- a/iocore/eventsystem/Makefile.am
+++ b/iocore/eventsystem/Makefile.am
@@ -59,6 +59,7 @@ libinkevent_a_SOURCES = \
P_UnixSocketManager.h \
P_VConnection.h \
P_VIO.h \
+ Continuation.cc \
Processor.cc \
ProtectedQueue.cc \
ProxyAllocator.cc \
diff --git a/iocore/eventsystem/UnixEThread.cc b/iocore/eventsystem/UnixEThread.cc
index 225d81f..b66b734 100644
--- a/iocore/eventsystem/UnixEThread.cc
+++ b/iocore/eventsystem/UnixEThread.cc
@@ -128,6 +128,7 @@ EThread::process_event(Event *e, int calling_code)
return;
}
Continuation *c_temp = e->continuation;
+ // Make sure that the contination is locked before calling the handler
e->continuation->handleEvent(calling_code, e);
ink_assert(!e->in_the_priority_queue);
ink_assert(c_temp == e->continuation);
diff --git a/iocore/net/UnixNetAccept.cc b/iocore/net/UnixNetAccept.cc
index 39714e1..fb451d7 100644
--- a/iocore/net/UnixNetAccept.cc
+++ b/iocore/net/UnixNetAccept.cc
@@ -126,7 +126,7 @@ net_accept(NetAccept *na, void *ep, bool blockable)
SET_CONTINUATION_HANDLER(vc, (NetVConnHandler)&UnixNetVConnection::acceptEvent);
if (e->ethread->is_event_type(na->opt.etype)) {
- vc->handleEvent(EVENT_NONE, e);
+ vc->dispatchEvent(EVENT_NONE, e);
} else {
eventProcessor.schedule_imm(vc, na->opt.etype);
}
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 3b12995..742b9be 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -1125,7 +1125,7 @@ UnixNetVConnection::acceptEvent(int event, Event *e)
UnixNetVConnection::set_active_timeout(active_timeout_in);
}
- action_.continuation->handleEvent(NET_EVENT_ACCEPT, this);
+ action_.continuation->dispatchEvent(NET_EVENT_ACCEPT, this);
return EVENT_DONE;
}
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index 0e4e13a..6e04efc 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1223,7 +1223,7 @@ HttpConfig::startup()
OutboundConnTrack::config_init(&c.outbound_conntrack, &c.oride.outbound_conntrack);
- http_config_cont->handleEvent(EVENT_NONE, nullptr);
+ http_config_cont->dispatchEvent(EVENT_NONE, nullptr);
return;
}
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 1c07d2d..1c055e6 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -534,7 +534,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
- handleEvent(VC_EVENT_READ_READY, ua_entry->read_vio);
+ dispatchEvent(VC_EVENT_READ_READY, ua_entry->read_vio);
}
void
@@ -866,7 +866,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.handleEvent(event, c->write_vio);
+ tunnel.dispatchEvent(event, c->write_vio);
return 0;
} else {
tunnel.kill_tunnel();
@@ -3954,7 +3954,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) {
- handleEvent(EVENT_REMAP_COMPLETE, nullptr);
+ dispatchEvent(EVENT_REMAP_COMPLETE, nullptr);
}
return;
}
@@ -5402,13 +5402,13 @@ HttpSM::handle_server_setup_error(int event, void *data)
ua_producer->alive = false;
ua_producer->handler_state = HTTP_SM_POST_SERVER_FAIL;
- tunnel.handleEvent(VC_EVENT_ERROR, c->write_vio);
+ tunnel.dispatchEvent(VC_EVENT_ERROR, c->write_vio);
return;
}
} else {
// c could be null here as well
if (c != nullptr) {
- tunnel.handleEvent(event, c->write_vio);
+ tunnel.dispatchEvent(event, c->write_vio);
return;
}
}
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index 9bac984..23cc2d4 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->handleEvent(HTTP_TUNNEL_EVENT_DONE, this);
+ sm->dispatchEvent(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->handleEvent(HTTP_TUNNEL_EVENT_DONE, this);
+ sm->dispatchEvent(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 6fef4fc..bd98152 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -1287,7 +1287,7 @@ APIHook::invoke(int event, void *edata)
ink_assert(!"not reached");
}
}
- return m_cont->handleEvent(event, edata);
+ return m_cont->dispatchEvent(event, edata);
}
APIHook *
@@ -4519,7 +4519,7 @@ int
TSContCall(TSCont contp, TSEvent event, void *edata)
{
Continuation *c = (Continuation *)contp;
- return c->handleEvent((int)event, edata);
+ return c->dispatchEvent((int)event, edata);
}
TSMutex