You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2016/10/20 17:02:55 UTC
[trafficserver] branch 7.0.x updated (1f1d708 -> 89befc8)
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a change to branch 7.0.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git.
from 1f1d708 Updated release notes
new 79cbf77 TS-4916 Add safety net to avoid H2-infinite-loop deadlock.
new 89befc8 Updated release notes
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "adds" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGES | 1 +
proxy/http2/Http2ConnectionState.cc | 17 ++++++++++++++---
proxy/http2/Http2ConnectionState.h | 2 +-
proxy/http2/Http2Stream.cc | 12 ++++++++++++
4 files changed, 28 insertions(+), 4 deletions(-)
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].
[trafficserver] 01/02: TS-4916 Add safety net to avoid
H2-infinite-loop deadlock.
Posted by bc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 7.0.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 79cbf777e21ff38186b13c99d0084233ba39607d
Author: Gancho Tenev <gt...@gmail.com>
AuthorDate: Mon Oct 17 15:10:12 2016 -0700
TS-4916 Add safety net to avoid H2-infinite-loop deadlock.
Current Http2ConnectionState implementation uses a memory pool for
instantiating streams and DLL<> stream_list for storing active streams.
Destroying a stream before deleting it from stream_list and then creating
a new one + reusing the same chunk from the memory pool right away always
leads to destroying the DLL structure (deadlocks, inconsistencies).
Added a safety net since the consequences are disastrous.
Until the design/implementation changes it seems less error prone
to (double) delete before destroying (noop if already deleted).
(cherry picked from commit a6f9337f61c980739e08bbefeb5f6409b7dcdac1)
---
proxy/http2/Http2ConnectionState.cc | 17 ++++++++++++++---
proxy/http2/Http2ConnectionState.h | 2 +-
proxy/http2/Http2Stream.cc | 12 ++++++++++++
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 1fd0f88..c14ebf3 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -908,6 +908,10 @@ Http2ConnectionState::create_stream(Http2StreamId new_id)
Http2Stream *new_stream = THREAD_ALLOC_INIT(http2StreamAllocator, this_ethread());
new_stream->init(new_id, client_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE));
+
+ ink_assert(NULL != new_stream);
+ ink_assert(!stream_list.in(new_stream));
+
stream_list.push(new_stream);
if (client_streamid) {
latest_streamid_in = new_id;
@@ -938,6 +942,7 @@ Http2ConnectionState::find_stream(Http2StreamId id) const
if (s->get_id() == id) {
return s;
}
+ ink_assert(s != s->link.next);
}
return NULL;
}
@@ -952,6 +957,7 @@ Http2ConnectionState::restart_streams()
if (!s->is_closed() && s->get_state() == HTTP2_STREAM_STATE_HALF_CLOSED_REMOTE && min(this->client_rwnd, s->client_rwnd) > 0) {
s->send_response_body();
}
+ ink_assert(s != next);
s = next;
}
}
@@ -963,9 +969,10 @@ Http2ConnectionState::cleanup_streams()
while (s) {
Http2Stream *next = s->link.next;
this->delete_stream(s);
+ ink_assert(s != next);
s = next;
}
- ink_assert(stream_list.head == NULL);
+ ink_assert(stream_list.empty());
if (!is_state_closed()) {
ua_session->get_netvc()->add_to_keep_alive_queue();
@@ -973,12 +980,14 @@ Http2ConnectionState::cleanup_streams()
}
}
-void
+bool
Http2ConnectionState::delete_stream(Http2Stream *stream)
{
+ ink_assert(NULL != stream);
+
// If stream has already been removed from the list, just go on
if (!stream_list.in(stream)) {
- return;
+ return false;
}
DebugHttp2Stream(ua_session, stream->get_id(), "Delete stream");
@@ -999,6 +1008,8 @@ Http2ConnectionState::delete_stream(Http2Stream *stream)
stream_list.remove(stream);
stream->initiating_close();
+
+ return true;
}
void
diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h
index eb6f930..7f52927 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -174,7 +174,7 @@ public:
Http2Stream *create_stream(Http2StreamId new_id);
Http2Stream *find_stream(Http2StreamId id) const;
void restart_streams();
- void delete_stream(Http2Stream *stream);
+ bool delete_stream(Http2Stream *stream);
void release_stream(Http2Stream *stream);
void cleanup_streams();
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 8b80d1b..4d3a07b 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -675,6 +675,18 @@ Http2Stream::destroy()
}
chunked_handler.clear();
super::destroy();
+
+ // Current Http2ConnectionState implementation uses a memory pool for instantiating streams and DLL<> stream_list for storing
+ // active streams. Destroying a stream before deleting it from stream_list and then creating a new one + reusing the same chunk
+ // from the memory pool right away always leads to destroying the DLL structure (deadlocks, inconsistencies).
+ // The following is meant as a safety net since the consequences are disastrous. Until the design/implementation changes it seems
+ // less error prone to (double) delete before destroying (noop if already deleted).
+ if (parent) {
+ if (static_cast<Http2ClientSession *>(parent)->connection_state.delete_stream(this)) {
+ Warning("Http2Stream was about to be deallocated without removing it from the active stream list");
+ }
+ }
+
THREAD_FREE(this, http2StreamAllocator, this_ethread());
}
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 02/02: Updated release notes
Posted by bc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 7.0.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
commit 89befc84075cfbbb13ce80756c837ca357cd54c9
Author: Bryan Call <bc...@apache.org>
AuthorDate: Thu Oct 20 10:01:08 2016 -0700
Updated release notes
---
CHANGES | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGES b/CHANGES
index cd18f9f..83585eb 100644
--- a/CHANGES
+++ b/CHANGES
@@ -377,6 +377,7 @@ Improvement:
*) [TS-4837] Add -Wextra to the build
*) [TS-4863] Remove non-existent proxy.node.num_processes metric.
*) [TS-4884] Remove a few unused variables/define in EThread
+ *) [TS-4909] Throttling based on resident memory usage
*) [TS-4931] Add process limits to crash logs
*) [TS-4949] Disable fuzzy revalidation logic
*) [TS-4959] Remove an old remnant of MSIE UA detection
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.