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 2015/06/29 23:38:03 UTC
trafficserver git commit: TS-3486: Crashes due to race condition on
server sessions moving between threads. This closes #235
Repository: trafficserver
Updated Branches:
refs/heads/master 315371ae9 -> a638d9701
TS-3486: Crashes due to race condition on server sessions moving between threads. This closes #235
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/a638d970
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/a638d970
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/a638d970
Branch: refs/heads/master
Commit: a638d97019e1b3ef4e6ec53b93d81a96885cbb6e
Parents: 315371a
Author: shinrich <sh...@yahoo-inc.com>
Authored: Fri Jun 26 13:11:40 2015 -0500
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Mon Jun 29 16:37:08 2015 -0500
----------------------------------------------------------------------
iocore/net/UnixNetVConnection.cc | 4 ++-
proxy/http/HttpServerSession.cc | 1 -
proxy/http/HttpSessionManager.cc | 56 ++++++++++++++++++-----------------
proxy/http/HttpSessionManager.h | 14 ++++-----
4 files changed, 39 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a638d970/iocore/net/UnixNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index ba96f24..1749ba6 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -247,7 +247,7 @@ read_from_net(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
MUTEX_TRY_LOCK_FOR(lock, s->vio.mutex, thread, s->vio._cont);
- if (!lock.is_locked() || lock.get_mutex() != s->vio.mutex.m_ptr) {
+ if (!lock.is_locked()) {
read_reschedule(nh, vc);
return;
}
@@ -637,9 +637,11 @@ UnixNetVConnection::do_io_close(int alerrno /* = -1 */)
read.vio.buffer.clear();
read.vio.nbytes = 0;
read.vio.op = VIO::NONE;
+ read.vio._cont = NULL;
write.vio.buffer.clear();
write.vio.nbytes = 0;
write.vio.op = VIO::NONE;
+ write.vio._cont = NULL;
EThread *t = this_ethread();
bool close_inline = !recursion && nh->mutex->thread_holding == t;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a638d970/proxy/http/HttpServerSession.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpServerSession.cc b/proxy/http/HttpServerSession.cc
index 5b71cfe..7acaa5e 100644
--- a/proxy/http/HttpServerSession.cc
+++ b/proxy/http/HttpServerSession.cc
@@ -170,7 +170,6 @@ HttpServerSession::release()
HSMresult_t r = httpSessionManager.release_session(this);
-
if (r == HSM_RETRY) {
// Session could not be put in the session manager
// due to lock contention
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a638d970/proxy/http/HttpSessionManager.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSessionManager.cc b/proxy/http/HttpSessionManager.cc
index c490515..80067d6 100644
--- a/proxy/http/HttpSessionManager.cc
+++ b/proxy/http/HttpSessionManager.cc
@@ -74,11 +74,10 @@ ServerSessionPool::match(HttpServerSession *ss, sockaddr const *addr, INK_MD5 co
(TS_SERVER_SESSION_SHARING_MATCH_HOST == match_style || ats_ip_addr_port_eq(ss->server_ip, addr));
}
-HttpServerSession *
-ServerSessionPool::acquireSession(sockaddr const *addr, INK_MD5 const &hostname_hash, TSServerSessionSharingMatchType match_style)
+HSMresult_t
+ServerSessionPool::acquireSession(sockaddr const *addr, INK_MD5 const &hostname_hash, TSServerSessionSharingMatchType match_style, HttpServerSession *&to_return)
{
- HttpServerSession *zret = NULL;
-
+ HSMresult_t zret = HSM_NOT_FOUND;
if (TS_SERVER_SESSION_SHARING_MATCH_HOST == match_style) {
// This is broken out because only in this case do we check the host hash first.
HostHashTable::Location loc = m_host_pool.find(hostname_hash);
@@ -86,9 +85,9 @@ ServerSessionPool::acquireSession(sockaddr const *addr, INK_MD5 const &hostname_
while (loc && port != ats_ip_port_cast(loc->server_ip))
++loc; // scan for matching port.
if (loc) {
- zret = loc;
+ to_return = loc;
m_host_pool.remove(loc);
- m_ip_pool.remove(m_ip_pool.find(zret));
+ m_ip_pool.remove(m_ip_pool.find(loc));
}
} else if (TS_SERVER_SESSION_SHARING_MATCH_NONE != match_style) { // matching is not disabled.
IPHashTable::Location loc = m_ip_pool.find(addr);
@@ -100,9 +99,9 @@ ServerSessionPool::acquireSession(sockaddr const *addr, INK_MD5 const &hostname_
++loc;
}
if (loc) {
- zret = loc;
+ to_return = loc;
m_ip_pool.remove(loc);
- m_host_pool.remove(m_host_pool.find(zret));
+ m_host_pool.remove(m_host_pool.find(loc));
}
}
return zret;
@@ -238,6 +237,7 @@ HttpSessionManager::acquire_session(Continuation * /* cont ATS_UNUSED */, sockad
TSServerSessionSharingMatchType match_style =
static_cast<TSServerSessionSharingMatchType>(sm->t_state.txn_conf->server_session_sharing_match);
INK_MD5 hostname_hash;
+ HSMresult_t retval = HSM_NOT_FOUND;
ink_code_md5((unsigned char *)hostname, strlen(hostname), (unsigned char *)&hostname_hash);
@@ -265,28 +265,30 @@ HttpSessionManager::acquire_session(Continuation * /* cont ATS_UNUSED */, sockad
// Now check to see if we have a connection in our shared connection pool
EThread *ethread = this_ethread();
- if (TS_SERVER_SESSION_SHARING_POOL_THREAD == sm->t_state.http_config_param->server_session_sharing_pool) {
- to_return = ethread->server_session_pool->acquireSession(ip, hostname_hash, match_style);
- } else {
- MUTEX_TRY_LOCK(lock, m_g_pool->mutex, ethread);
- if (lock.is_locked()) {
- to_return = m_g_pool->acquireSession(ip, hostname_hash, match_style);
- Debug("http_ss", "[acquire session] pool search %s", to_return ? "successful" : "failed");
+ ProxyMutex * pool_mutex = (TS_SERVER_SESSION_SHARING_POOL_THREAD == sm->t_state.http_config_param->server_session_sharing_pool) ? ethread->server_session_pool->mutex : m_g_pool->mutex;
+ MUTEX_TRY_LOCK(lock, pool_mutex, ethread);
+ if (lock.is_locked()) {
+ if (TS_SERVER_SESSION_SHARING_POOL_THREAD == sm->t_state.http_config_param->server_session_sharing_pool) {
+ retval = ethread->server_session_pool->acquireSession(ip, hostname_hash, match_style, to_return);
+ Debug("http_ss", "[acquire session] thread pool search %s", to_return ? "successful" : "failed");
} else {
- Debug("http_ss", "[acquire session] could not acquire session due to lock contention");
- return HSM_RETRY;
+ retval = m_g_pool->acquireSession(ip, hostname_hash, match_style, to_return);
+ Debug("http_ss", "[acquire session] global pool search %s", to_return ? "successful" : "failed");
}
+ if (to_return) {
+ Debug("http_ss", "[%" PRId64 "] [acquire session] return session from shared pool", to_return->con_id);
+ to_return->state = HSS_ACTIVE;
+ // Holding the pool lock and the sm lock
+ // the attach_server_session will issue the do_io_read under the sm lock
+ // Must be careful to transfer the lock for the read vio because
+ // the server VC may be moving between threads TS-3266
+ sm->attach_server_session(to_return);
+ retval = HSM_DONE;
+ }
+ } else {
+ retval = HSM_RETRY;
}
-
- if (to_return) {
- Debug("http_ss", "[%" PRId64 "] [acquire session] "
- "return session from shared pool",
- to_return->con_id);
- to_return->state = HSS_ACTIVE;
- sm->attach_server_session(to_return);
- return HSM_DONE;
- }
- return HSM_NOT_FOUND;
+ return retval;
}
HSMresult_t
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a638d970/proxy/http/HttpSessionManager.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSessionManager.h b/proxy/http/HttpSessionManager.h
index 07462bb..a95ba56 100644
--- a/proxy/http/HttpSessionManager.h
+++ b/proxy/http/HttpSessionManager.h
@@ -43,6 +43,12 @@ class HttpSM;
void initialize_thread_for_http_sessions(EThread *thread, int thread_index);
+enum HSMresult_t {
+ HSM_DONE,
+ HSM_RETRY,
+ HSM_NOT_FOUND,
+};
+
/** A pool of server sessions.
This is a continuation so that it can get callbacks from the server sessions.
@@ -126,7 +132,7 @@ public:
@return A pointer to the session or @c NULL if not matching session was found.
*/
- HttpServerSession *acquireSession(sockaddr const *addr, INK_MD5 const &host_hash, TSServerSessionSharingMatchType match_style);
+ HSMresult_t acquireSession(sockaddr const *addr, INK_MD5 const &host_hash, TSServerSessionSharingMatchType match_style, HttpServerSession *&server_session);
/** Release a session to to pool.
*/
void releaseSession(HttpServerSession *ss);
@@ -140,12 +146,6 @@ public:
HostHashTable m_host_pool;
};
-enum HSMresult_t {
- HSM_DONE,
- HSM_RETRY,
- HSM_NOT_FOUND,
-};
-
class HttpSessionManager
{
public: