You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2018/01/22 23:26:59 UTC
[trafficserver] branch 7.1.x updated (651d917 -> e575b7d)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a change to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.
from 651d917 clang-format
new 0d37a50 Address session/transaction close errors.
new e575b7d Fix session-based memory leak.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
proxy/ProxyClientTransaction.cc | 5 +-
proxy/ProxyClientTransaction.h | 4 --
proxy/http/Http1ClientSession.cc | 34 +++++++------
proxy/http/Http1ClientSession.h | 3 ++
proxy/http/Http1ClientTransaction.cc | 14 +-----
proxy/http/Http1ClientTransaction.h | 94 +++++++++++++++++++-----------------
6 files changed, 75 insertions(+), 79 deletions(-)
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 02/02: Fix session-based memory leak.
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit e575b7d2d4d5d7c823608129bee37ac6879998b7
Author: Susan Hinrichs <sh...@ieee.org>
AuthorDate: Wed Jun 7 17:49:40 2017 +0000
Fix session-based memory leak.
(cherry picked from commit 63b34dc5bfc7e3e1eaf796056754b675796452d2)
---
proxy/ProxyClientTransaction.h | 4 --
proxy/http/Http1ClientSession.cc | 2 +-
proxy/http/Http1ClientTransaction.cc | 10 ----
proxy/http/Http1ClientTransaction.h | 94 +++++++++++++++++++-----------------
4 files changed, 51 insertions(+), 59 deletions(-)
diff --git a/proxy/ProxyClientTransaction.h b/proxy/ProxyClientTransaction.h
index b5735f8..f0004f6 100644
--- a/proxy/ProxyClientTransaction.h
+++ b/proxy/ProxyClientTransaction.h
@@ -171,10 +171,6 @@ public:
set_outbound_ip(const IpAddr &new_addr)
{
}
- virtual void
- clear_outbound()
- {
- }
virtual bool
is_outbound_transparent() const
{
diff --git a/proxy/http/Http1ClientSession.cc b/proxy/http/Http1ClientSession.cc
index be0d927..04bfa35 100644
--- a/proxy/http/Http1ClientSession.cc
+++ b/proxy/http/Http1ClientSession.cc
@@ -134,7 +134,7 @@ Http1ClientSession::free()
this->do_io_write(nullptr, 0, nullptr);
// Free the transaction resources
- this->trans.cleanup();
+ this->trans.super::destroy();
super::free();
THREAD_FREE(this, http1ClientSessionAllocator, this_thread());
diff --git a/proxy/http/Http1ClientTransaction.cc b/proxy/http/Http1ClientTransaction.cc
index f6808a0..9c662c6 100644
--- a/proxy/http/Http1ClientTransaction.cc
+++ b/proxy/http/Http1ClientTransaction.cc
@@ -63,17 +63,7 @@ Http1ClientTransaction::set_parent(ProxyClientSession *new_parent)
void
Http1ClientTransaction::transaction_done()
{
- // If the parent session is not in the closed state, the destroy will not occur.
if (parent) {
static_cast<Http1ClientSession *>(parent)->release_transaction();
}
}
-
-void
-Http1ClientTransaction::destroy()
-{
- if (current_reader) {
- current_reader->ua_session = nullptr;
- current_reader = nullptr;
- }
-}
diff --git a/proxy/http/Http1ClientTransaction.h b/proxy/http/Http1ClientTransaction.h
index 8fa7319..7547f6b 100644
--- a/proxy/http/Http1ClientTransaction.h
+++ b/proxy/http/Http1ClientTransaction.h
@@ -35,87 +35,93 @@ public:
Http1ClientTransaction() : super(), outbound_port(0), outbound_transparent(false) {}
// Implement VConnection interface.
- virtual VIO *
- do_io_read(Continuation *c, int64_t nbytes = INT64_MAX, MIOBuffer *buf = 0)
+ VIO *
+ do_io_read(Continuation *c, int64_t nbytes = INT64_MAX, MIOBuffer *buf = 0) override
{
return parent->do_io_read(c, nbytes, buf);
}
- virtual VIO *
- do_io_write(Continuation *c = NULL, int64_t nbytes = INT64_MAX, IOBufferReader *buf = 0, bool owner = false)
+ VIO *
+ do_io_write(Continuation *c = NULL, int64_t nbytes = INT64_MAX, IOBufferReader *buf = 0, bool owner = false) override
{
return parent->do_io_write(c, nbytes, buf, owner);
}
- virtual void
- do_io_close(int lerrno = -1)
+ void
+ do_io_close(int lerrno = -1) override
{
parent->do_io_close(lerrno);
// this->destroy(); Parent owns this data structure. No need for separate destroy.
}
// Don't destroy your elements. Rely on the Http1ClientSession to clean up the
- // Http1ClientTransaction class as necessary
- virtual void destroy();
-
- // Clean up the transaction elements when the ClientSession shuts down
+ // Http1ClientTransaction class as necessary. The super::destroy() clears the
+ // mutex, which Http1ClientSession owns.
void
- cleanup()
+ destroy() override
{
- super::destroy();
+ current_reader = nullptr;
}
- virtual void
- do_io_shutdown(ShutdownHowTo_t howto)
+ void
+ do_io_shutdown(ShutdownHowTo_t howto) override
{
parent->do_io_shutdown(howto);
}
- virtual void
- reenable(VIO *vio)
+
+ void
+ reenable(VIO *vio) override
{
parent->reenable(vio);
}
+
void
set_reader(IOBufferReader *reader)
{
sm_reader = reader;
}
- void release(IOBufferReader *r);
- virtual bool
- ignore_keep_alive()
+
+ void release(IOBufferReader *r) override;
+
+ bool
+ ignore_keep_alive() override
{
return false;
}
- virtual bool
- allow_half_open() const
+ bool
+ allow_half_open() const override
{
return true;
}
- void set_parent(ProxyClientSession *new_parent);
+ void set_parent(ProxyClientSession *new_parent) override;
- virtual uint16_t
- get_outbound_port() const
+ uint16_t
+ get_outbound_port() const override
{
return outbound_port;
}
- virtual IpAddr
- get_outbound_ip4() const
+
+ IpAddr
+ get_outbound_ip4() const override
{
return outbound_ip4;
}
- virtual IpAddr
- get_outbound_ip6() const
+
+ IpAddr
+ get_outbound_ip6() const override
{
return outbound_ip6;
}
- virtual void
- set_outbound_port(uint16_t new_port)
+
+ void
+ set_outbound_port(uint16_t new_port) override
{
outbound_port = new_port;
}
- virtual void
- set_outbound_ip(const IpAddr &new_addr)
+
+ void
+ set_outbound_ip(const IpAddr &new_addr) override
{
if (new_addr.isIp4()) {
outbound_ip4 = new_addr;
@@ -125,43 +131,43 @@ public:
clear_outbound_ip();
}
}
- virtual void
+ void
clear_outbound_ip()
{
outbound_ip4.invalidate();
outbound_ip6.invalidate();
}
- virtual bool
- is_outbound_transparent() const
+ bool
+ is_outbound_transparent() const override
{
return outbound_transparent;
}
- virtual void
- set_outbound_transparent(bool flag)
+ void
+ set_outbound_transparent(bool flag) override
{
outbound_transparent = flag;
}
// Pass on the timeouts to the netvc
- virtual void
- set_active_timeout(ink_hrtime timeout_in)
+ void
+ set_active_timeout(ink_hrtime timeout_in) override
{
if (parent)
parent->set_active_timeout(timeout_in);
}
- virtual void
- set_inactivity_timeout(ink_hrtime timeout_in)
+ void
+ set_inactivity_timeout(ink_hrtime timeout_in) override
{
if (parent)
parent->set_inactivity_timeout(timeout_in);
}
- virtual void
- cancel_inactivity_timeout()
+ void
+ cancel_inactivity_timeout() override
{
if (parent)
parent->cancel_inactivity_timeout();
}
- void transaction_done();
+ void transaction_done() override;
protected:
uint16_t outbound_port;
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.
[trafficserver] 01/02: Address session/transaction close errors.
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 0d37a500dd34cb0f740c65dbc21426c38015f443
Author: Susan Hinrichs <sh...@draggingnagging.corp.ne1.yahoo.com>
AuthorDate: Thu Oct 20 23:35:18 2016 +0000
Address session/transaction close errors.
(cherry picked from commit 54528f7a6daa749a529db4475b553ac7d81d17ff)
Conflicts:
proxy/http2/Http2Stream.cc
---
proxy/ProxyClientTransaction.cc | 5 +----
proxy/http/Http1ClientSession.cc | 32 +++++++++++++++++++-------------
proxy/http/Http1ClientSession.h | 3 +++
proxy/http/Http1ClientTransaction.cc | 4 +---
4 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/proxy/ProxyClientTransaction.cc b/proxy/ProxyClientTransaction.cc
index 44bed51..ad03961 100644
--- a/proxy/ProxyClientTransaction.cc
+++ b/proxy/ProxyClientTransaction.cc
@@ -81,10 +81,7 @@ ProxyClientTransaction::attach_server_session(HttpServerSession *ssession, bool
void
ProxyClientTransaction::destroy()
{
- if (current_reader) {
- current_reader->ua_session = nullptr;
- current_reader = nullptr;
- }
+ current_reader = nullptr;
this->mutex.clear();
}
diff --git a/proxy/http/Http1ClientSession.cc b/proxy/http/Http1ClientSession.cc
index 2092bd8..be0d927 100644
--- a/proxy/http/Http1ClientSession.cc
+++ b/proxy/http/Http1ClientSession.cc
@@ -75,6 +75,7 @@ Http1ClientSession::Http1ClientSession()
ka_vio(nullptr),
slave_ka_vio(nullptr),
bound_ss(nullptr),
+ released_transactions(0),
outbound_port(0),
f_outbound_transparent(false),
f_transparent_passthrough(false)
@@ -89,12 +90,23 @@ Http1ClientSession::destroy()
}
if (!in_destroy) {
in_destroy = true;
- DebugHttpSsn("[%" PRId64 "] session destroy", con_id);
+ DebugHttpSsn("[%" PRId64 "] session destroy", con_id);
ink_release_assert(!client_vc);
ink_assert(read_buffer);
-
+ ink_release_assert(transact_count == released_transactions);
do_api_callout(TS_HTTP_SSN_CLOSE_HOOK);
+ } else {
+ Warning("http1: Attempt to double ssn close");
+ }
+}
+
+void
+Http1ClientSession::release_transaction()
+{
+ released_transactions++;
+ if (transact_count == released_transactions) {
+ destroy();
}
}
@@ -267,7 +279,10 @@ Http1ClientSession::do_io_close(int alerrno)
bound_ss = nullptr;
slave_ka_vio = nullptr;
}
-
+ // Completed the last transaction. Just shutdown already
+ if (transact_count == released_transactions) {
+ half_close = false;
+ }
if (half_close && this->trans.get_sm()) {
read_state = HCS_HALF_CLOSED;
SET_HANDLER(&Http1ClientSession::state_wait_for_close);
@@ -306,7 +321,7 @@ Http1ClientSession::do_io_close(int alerrno)
client_vc = nullptr;
}
}
- if (trans.get_sm() == nullptr) { // Destroying from keep_alive state
+ if (transact_count == released_transactions) {
this->destroy();
}
}
@@ -406,16 +421,7 @@ Http1ClientSession::state_keep_alive(int event, void *data)
break;
case VC_EVENT_EOS:
- // If there is data in the buffer, start a new
- // transaction, otherwise the client gave up
- // SKH - A bit odd starting a transaction when the client has closed
- // already. At a minimum, should have to do some half open connection
- // tracking
- // if (sm_reader->read_avail() > 0) {
- // new_transaction();
- //} else {
this->do_io_close();
- //}
break;
case VC_EVENT_READ_COMPLETE:
diff --git a/proxy/http/Http1ClientSession.h b/proxy/http/Http1ClientSession.h
index a2321b4..926e4b0 100644
--- a/proxy/http/Http1ClientSession.h
+++ b/proxy/http/Http1ClientSession.h
@@ -57,6 +57,7 @@ public:
// Implement ProxyClientSession interface.
virtual void destroy();
virtual void free();
+ void release_transaction();
virtual void
start()
@@ -213,6 +214,8 @@ private:
HttpServerSession *bound_ss;
+ int released_transactions;
+
public:
// Link<Http1ClientSession> debug_link;
LINK(Http1ClientSession, debug_link);
diff --git a/proxy/http/Http1ClientTransaction.cc b/proxy/http/Http1ClientTransaction.cc
index 05eef7b..f6808a0 100644
--- a/proxy/http/Http1ClientTransaction.cc
+++ b/proxy/http/Http1ClientTransaction.cc
@@ -63,11 +63,9 @@ Http1ClientTransaction::set_parent(ProxyClientSession *new_parent)
void
Http1ClientTransaction::transaction_done()
{
- current_reader = nullptr;
// If the parent session is not in the closed state, the destroy will not occur.
if (parent) {
- parent->destroy();
- parent = nullptr;
+ static_cast<Http1ClientSession *>(parent)->release_transaction();
}
}
--
To stop receiving notification emails like this one, please contact
zwoop@apache.org.