You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2012/05/16 06:15:26 UTC
[3/3] git commit: TS-934: backout earlier patch in favor of
subsequent patches.
TS-934: backout earlier patch in favor of subsequent patches.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/72ef2def
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/72ef2def
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/72ef2def
Branch: refs/heads/master
Commit: 72ef2def62ed66b2827a2959ab7e893544b07376
Parents: bc96275
Author: jplevyak@apache.org <jp...@apache.org>
Authored: Tue May 15 21:10:38 2012 -0700
Committer: John Plevyak <jp...@acm.org>
Committed: Tue May 15 21:11:04 2012 -0700
----------------------------------------------------------------------
iocore/net/I_NetVConnection.h | 84 ---------------------------------
iocore/net/NetVConnection.cc | 61 ------------------------
iocore/net/UnixNetVConnection.cc | 1 -
proxy/http/HttpServerSession.cc | 4 +-
proxy/http/HttpServerSession.h | 4 +-
5 files changed, 4 insertions(+), 150 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/72ef2def/iocore/net/I_NetVConnection.h
----------------------------------------------------------------------
diff --git a/iocore/net/I_NetVConnection.h b/iocore/net/I_NetVConnection.h
index 0c55580..ae78380 100644
--- a/iocore/net/I_NetVConnection.h
+++ b/iocore/net/I_NetVConnection.h
@@ -433,7 +433,6 @@ public:
unsigned int attributes;
EThread *thread;
- volatile uint32_t generation; ///< Counter for re-use checking.
/// PRIVATE: The public interface is VIO::reenable()
virtual void reenable(VIO * vio) = 0;
@@ -482,88 +481,6 @@ public:
is_transparent = state;
}
- /** Struct for holding a reference to a connection.
- The problem is that connections are accessed across thread boundaries
- and re-used so that a pointer to a VC can become not only stale
- but a reference to another connection entirely aynchronously. The
- lock is contained within the connection so it can't be reliably accessed
- from just a raw pointer.
- */
- struct Handle {
- typedef Handle self; ///< Self reference type.
-
- NetVConnection* _vc; ///< Pointer to the VC.
- uint32_t _generation; ///< Generation of VC when assigned.
- ProxyMutexPtr _mutex; ///< Shared mutex with VC at time of assignment.
-
- /// Default constructor.
- Handle() : _vc(0), _generation(0) {}
- /// Construct from VC.
- Handle(NetVConnection* vc) { *this = vc; }
- /// Assign VC.
- self& operator = (NetVConnection* vc) {
- _vc = vc;
- if (vc) {
- _mutex = vc->mutex;
- _generation = vc->generation;
- } else {
- _generation = 0;
- _mutex.clear();
- }
- return *this;
- }
-
- /// Dereference operator.
- NetVConnection* operator -> () { return _vc; }
- /// Get referenced VC.
- NetVConnection* getVC() { return _vc; }
- /// Get pointer mutex.
- ProxyMutexPtr& getMutex() { return _mutex; }
- /** Check if this reference is still valid.
- This should only be called under lock of @a _mutex.
- @return @c true
- */
- bool isValid() {
- return _vc && // have a connection
- _mutex.m_ptr == _vc->mutex.m_ptr && // and it's still the same mutex
- _generation == _vc->generation // and same generation.
- ;
- }
- /** Invoke @c do_io_close on the underlying VC.
- This handles locking and validating the VC. If a lock can be obtained
- then the operation is performed immediately. If not then it is
- scheduled on the thread holding the lock.
- */
- void do_locked_io_close(int lerrno = -1);
- };
-
- /** A subclass of @c Handle that can be used in @c IntrusivePtrList.
- */
-
- /** Use this to schedule closing the connection.
-
- This can be needed if the connection is part of a transaction
- but being handled in another thread. The continuation mutex is a
- shared reference to the VC mutex so that it is not garbage
- collected and can be checked against the VC after being locked.
- */
- struct Close_callback : public Continuation {
- typedef Continuation super; ///< Super type.
- typedef Close_callback self; ///< Self reference type.
-
- Handle _vcptr; ///< Reference to the target VC.
- int _lerrno; ///< Errno value to pass to @c do_io_close.
-
- int try_close(int, Event *);
-
- Close_callback(Handle const& vcptr, int lerrno = -1)
- : super(&*(vcptr._mutex))
- , _vcptr(vcptr)
- , _lerrno(lerrno) {
- SET_HANDLER(&self::try_close);
- }
- };
-
private:
NetVConnection(const NetVConnection &);
NetVConnection & operator =(const NetVConnection &);
@@ -585,7 +502,6 @@ NetVConnection::NetVConnection():
VConnection(NULL),
attributes(0),
thread(NULL),
- generation(1),
got_local_addr(0),
got_remote_addr(0),
is_internal_request(false),
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/72ef2def/iocore/net/NetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/NetVConnection.cc b/iocore/net/NetVConnection.cc
index 98c5f2f..1050a47 100644
--- a/iocore/net/NetVConnection.cc
+++ b/iocore/net/NetVConnection.cc
@@ -44,64 +44,3 @@ NetVConnection::cancel_OOB()
return;
}
-int
-NetVConnection::Close_callback::try_close(int, Event *) {
- // Mutex for this continuation is the original VC mutex so if we're
- // being dispatched, it should be locked enough to check.
- if (_vcptr.isValid()) {
- _vcptr->do_io_close(_lerrno);
- }
- delete this;
- return EVENT_DONE;
-}
-
-void
-NetVConnection::Handle::do_locked_io_close(int lerrno) {
- EThread* t = this_ethread();
- // If the VC or the mutex isn't there, we're done.
- bool handled = ! (_vc && _mutex.m_ptr);
- /* This loop is an attempt to avoid race conditions. We need to
- shut down the VC without colliding with another thread.
-
- First we try to get the lock. If that works then if our
- reference is still valid we can do the shutdown directly. If
- it's not valid we assume that someone else already did the
- shutdown (such a collision being precisely what we're trying to
- avoid).
-
- If we can't get the lock, we need to schedule a retry on the
- thread that owns the lock. However, we're racing at this because
- without the lock the VC or the lock itself can change. In
- particular, the thread owning the lock can drop it and leave us
- without a valid thread. So we grab what we can and check it. If
- we have a thread, we schedule there. Otherwise the lock has been
- dropped so we cycle back to trying the lock.
-
- We presume that there's extremely little chance of this just
- missing the lock happening and an infinitesmal chance of it
- happening more than once. So the loop shouldn't go more than once
- or twice and then only vary rarely. Further, based on experience
- in a high throughput environment we should expect to not get the
- lock in the first place only in rare circumstances. For most
- situations and in most environments we should not ever do the
- scheduling. Unfortunately it does happen under some circumstances
- so we have to deal with it.
- */
- while (!handled) {
- MUTEX_TRY_LOCK(lock, _mutex, t);
- if (lock) {
- if (this->isValid()) _vc->do_io_close(lerrno);
- handled = true;
- } else {
- // Gosh, it's probable that the VC has gone bad at this point but
- // we can't reliably detect that without the lock.
- EThread* ot = _mutex->thread_holding;
- ink_assert(ot != t); // we should have the lock in that case.
- if (ot) {
- // schedule shutdown on other thread.
- ot->schedule_imm(NEW(new UnixNetVConnection::Close_callback(*this, lerrno)));
- handled = true;
- }
- }
- }
-}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/72ef2def/iocore/net/UnixNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index b74defa..adaff36 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -1127,7 +1127,6 @@ UnixNetVConnection::free(EThread *t)
{
NET_SUM_GLOBAL_DYN_STAT(net_connections_currently_open_stat, -1);
// clear variables for reuse
- ++generation;
this->mutex.clear();
got_remote_addr = 0;
got_local_addr = 0;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/72ef2def/proxy/http/HttpServerSession.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpServerSession.cc b/proxy/http/HttpServerSession.cc
index 60783eb..5ff532e 100644
--- a/proxy/http/HttpServerSession.cc
+++ b/proxy/http/HttpServerSession.cc
@@ -41,7 +41,7 @@ ClassAllocator<HttpServerSession> httpServerSessionAllocator("httpServerSessionA
void
HttpServerSession::destroy()
{
- ink_release_assert(server_vc.getVC() == NULL);
+ ink_release_assert(server_vc == NULL);
ink_assert(read_buffer);
ink_assert(server_trans_stat == 0);
magic = HTTP_SS_MAGIC_DEAD;
@@ -119,7 +119,7 @@ HttpServerSession::do_io_close(int alerrno)
this->server_trans_stat--;
}
- server_vc.do_locked_io_close(alerrno);
+ server_vc->do_io_close(alerrno);
Debug("http_ss", "[%" PRId64 "] session closed", con_id);
server_vc = NULL;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/72ef2def/proxy/http/HttpServerSession.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpServerSession.h b/proxy/http/HttpServerSession.h
index 8078158..89db5bd 100644
--- a/proxy/http/HttpServerSession.h
+++ b/proxy/http/HttpServerSession.h
@@ -110,7 +110,7 @@ public:
void attach_hostname(const char *hostname);
NetVConnection *get_netvc()
{
- return server_vc.getVC();
+ return server_vc;
};
// Keys for matching hostnames
@@ -160,7 +160,7 @@ public:
private:
HttpServerSession(HttpServerSession &);
- NetVConnection::Handle server_vc;
+ NetVConnection *server_vc;
int magic;
IOBufferReader *buf_reader;