You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2017/01/11 02:24:03 UTC
mesos git commit: Eliminated an EOF race condition in libprocess SSL
socket.
Repository: mesos
Updated Branches:
refs/heads/master bea98b9b9 -> d4185d4a1
Eliminated an EOF race condition in libprocess SSL socket.
Previously, it was possible for an SSL socket to either:
1) Fail to receive an EOF if the EOF event was received when
there was no pending recv() request.
2) Fail to receive all data sent on the sending side if an
EOF event was received before all sent data was read.
This patch eliminates these race conditions to ensure reliable
receipt of both sent data and EOFs.
Review: https://reviews.apache.org/r/53802/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d4185d4a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d4185d4a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d4185d4a
Branch: refs/heads/master
Commit: d4185d4a117d9cba32d80993d14fcefb566cd9f9
Parents: bea98b9
Author: Greg Mann <gr...@mesosphere.io>
Authored: Tue Jan 10 18:23:45 2017 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Tue Jan 10 18:23:45 2017 -0800
----------------------------------------------------------------------
3rdparty/libprocess/src/libevent_ssl_socket.cpp | 56 ++++++++++++++------
3rdparty/libprocess/src/libevent_ssl_socket.hpp | 5 ++
2 files changed, 44 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4185d4a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
index dddd0e2..b0319b2 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -249,23 +249,22 @@ void LibeventSSLSocketImpl::recv_callback()
// g. libevent callback is called for the event queued at step b.
// h. libevent callback finds the length of the buffer as 0 but the request is
// a non-nullptr due to step f.
- if (buffer_length > 0) {
+ if (buffer_length > 0 || received_eof) {
synchronized (lock) {
std::swap(request, recv_request);
}
}
if (request.get() != nullptr) {
- // There is an invariant that if we are executing a
- // 'recv_callback' and we have a request there must be data here
- // because we should not be getting a spurrious receive callback
- // invocation. Even if we discarded a request, the manual
- // invocation of 'recv_callback' guarantees that there is a
- // non-zero amount of data available in the bufferevent.
- size_t length = bufferevent_read(bev, request->data, request->size);
- CHECK(length > 0);
-
- request->promise.set(length);
+ if (buffer_length > 0) {
+ size_t length = bufferevent_read(bev, request->data, request->size);
+ CHECK(length > 0);
+
+ request->promise.set(length);
+ } else {
+ CHECK(received_eof);
+ request->promise.set(0);
+ }
}
}
@@ -360,11 +359,33 @@ void LibeventSSLSocketImpl::event_callback(short events)
// either because it was never created, it has already been
// completed, or it has been discarded.
+ // The case below where `EVUTIL_SOCKET_ERROR() == 0` will catch
+ // unclean shutdowns of the socket.
+ //
+ // TODO(greggomann): We should make use of the `BEV_EVENT_READING`
+ // and `BEV_EVENT_WRITING` flags to handle read and write errors
+ // differently. Related JIRA: MESOS-6770
if (events & BEV_EVENT_EOF ||
(events & BEV_EVENT_ERROR && EVUTIL_SOCKET_ERROR() == 0)) {
// At end of file, close the connection.
if (current_recv_request.get() != nullptr) {
- current_recv_request->promise.set(0);
+ received_eof = true;
+ // Drain any remaining data from the bufferevent or complete the
+ // promise with 0 to signify EOF. Because we set `received_eof`,
+ // subsequent calls to `recv` will return 0 if there is no data
+ // remaining on the buffer.
+ if (evbuffer_get_length(bufferevent_get_input(bev)) > 0) {
+ size_t length =
+ bufferevent_read(
+ bev,
+ current_recv_request->data,
+ current_recv_request->size);
+ CHECK(length > 0);
+
+ current_recv_request->promise.set(length);
+ } else {
+ current_recv_request->promise.set(0);
+ }
}
if (current_send_request.get() != nullptr) {
@@ -659,11 +680,12 @@ Future<size_t> LibeventSSLSocketImpl::recv(char* data, size_t size)
evbuffer* input = bufferevent_get_input(self->bev);
size_t length = evbuffer_get_length(input);
- // If there is already data in the buffer, fulfill the
- // 'recv_request' by calling 'recv_callback()'. Otherwise
- // do nothing and wait for the 'recv_callback' to run when
- // we receive data over the network.
- if (length > 0) {
+ // If there is already data in the buffer or an EOF has
+ // been received, fulfill the 'recv_request' by calling
+ // 'recv_callback()'. Otherwise do nothing and wait for
+ // the 'recv_callback' to run when we receive data over
+ // the network.
+ if (length > 0 || self->received_eof) {
self->recv_callback();
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4185d4a/3rdparty/libprocess/src/libevent_ssl_socket.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.hpp b/3rdparty/libprocess/src/libevent_ssl_socket.hpp
index 57eaf4f..65da091 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.hpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.hpp
@@ -150,6 +150,11 @@ private:
Owned<SendRequest> send_request;
Owned<ConnectRequest> connect_request;
+ // Indicates whether or not an EOF has been received on this socket.
+ // Our accesses to this member are not synchronized because they all
+ // occur within the event loop, which runs on a single thread.
+ bool received_eof = false;
+
// This is a weak pointer to 'this', i.e., ourselves, this class
// instance. We need this for our event loop callbacks because it's
// possible that we'll actually want to cleanup this socket impl