[1/5] mesos git commit: Removed incorrect CHECK in SSL socket `send()`.

[josephwu]

Repository: mesos
Updated Branches:
  refs/heads/0.28.x a82539ff2 -> 91845ec22


Removed incorrect CHECK in SSL socket `send()`.

The lambda placed on the event loop by the libevent SSL
socket's `send()` method previously used a `CHECK` to
ensure that the socket's `send_request` member was not
`nullptr`. This patch removes this check, since
`send_request` may become `nullptr` any time the socket
receives an EOF or ERROR event.

Note that the current handling of events is incorrect
also, but we do not attempt a fix here. To be specific,
reading EOF should not deal with send requests at all
(see MESOS-5999). Also, the ERROR events are not
differentiated between reading and writing. Lastly,
when we receive an EOF we do not ensure that the caller
can read the bytes that remain in the buffer!

Review: https://reviews.apache.org/r/50741/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/237f316a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/237f316a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/237f316a

Branch: refs/heads/0.28.x
Commit: 237f316a18f4a7b33e11e7ea2af0e118caed3c21
Parents: a82539f
Author: Greg Mann 
Authored: Fri Aug 5 18:19:33 2016 -0700
Committer: Joseph Wu 
Committed: Thu Sep 15 10:18:40 2016 -0700

--
 3rdparty/libprocess/src/libevent_ssl_socket.cpp | 48 ++--
 1 file changed, 34 insertions(+), 14 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/237f316a/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 264df48..77af7b3 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -337,6 +337,12 @@ void LibeventSSLSocketImpl::event_callback(short events)
   // In all of the following conditions, we're interested in swapping
   // the value of the requests with null (if they are already null,
   // then there's no harm).
+  //
+  // TODO(bmahler): If we receive an EOF because the receiving
+  // side only shutdown writes on its socket, we can technically
+  // still send data on the socket!
+  //   See: http://www.unixguide.net/network/socketfaq/2.6.shtml
+  //   Related JIRA: MESOS-5999
   if (events & BEV_EVENT_EOF ||
   events & BEV_EVENT_CONNECTED ||
   events & BEV_EVENT_ERROR) {
@@ -692,14 +698,21 @@ Future LibeventSSLSocketImpl::send(const char* 
data, size_t size)
 CHECK(__in_event_loop__);
 CHECK(self);
 
-// We check that send_request is valid, because we do not
-// allow discards. This means there is no race between the
-// entry of 'send' and the execution of this lambda.
+// Check if the socket is closed or the write end has
+// encountered an error in the interim (i.e. we received
+// a BEV_EVENT_ERROR with BEV_EVENT_WRITING).
+bool write = false;
+
 synchronized (self->lock) {
-  CHECK_NOTNULL(self->send_request.get());
+  if (self->send_request.get() != nullptr) {
+write = true;
+  }
 }
 
-bufferevent_write(self->bev, data, size);
+if (write) {
+  int result = bufferevent_write(self->bev, data, size);
+  CHECK_EQ(0, result);
+}
   },
   DISALLOW_SHORT_CIRCUIT);
 
@@ -736,18 +749,25 @@ Future LibeventSSLSocketImpl::sendfile(
 CHECK(__in_event_loop__);
 CHECK(self);
 
-// We check that send_request is valid, because we do not
-// allow discards. This means there is no race between the
-// entry of 'sendfile' and the execution of this lambda.
+// Check if the socket is closed or the write end has
+// encountered an error in the interim (i.e. we received
+// a BEV_EVENT_ERROR with BEV_EVENT_WRITING).
+bool write = false;
+
 synchronized (self->lock) {
-  CHECK_NOTNULL(self->send_request.get());
+  if (self->send_request.get() != nullptr) {
+write = true;
+  }
 }
 
-evbuffer_add_file(
-bufferevent_get_output(self->bev),
-fd,
-offset,
-size);
+if (write) {
+  int result = evbuffer_add_file(
+  bufferevent_get_output(self->bev),
+  fd,
+  offset,
+  size);
+  CHECK_EQ(0, result);
+}
   },
   DISALLOW_SHORT_CIRCUIT);
 

[1/5] mesos git commit: Removed incorrect CHECK in SSL socket `send()`.

[josephwu]

Repository: mesos
Updated Branches:
  refs/heads/0.27.x 329aca4a9 -> ed0a8762e


Removed incorrect CHECK in SSL socket `send()`.

The lambda placed on the event loop by the libevent SSL
socket's `send()` method previously used a `CHECK` to
ensure that the socket's `send_request` member was not
`nullptr`. This patch removes this check, since
`send_request` may become `nullptr` any time the socket
receives an EOF or ERROR event.

Note that the current handling of events is incorrect
also, but we do not attempt a fix here. To be specific,
reading EOF should not deal with send requests at all
(see MESOS-5999). Also, the ERROR events are not
differentiated between reading and writing. Lastly,
when we receive an EOF we do not ensure that the caller
can read the bytes that remain in the buffer!

Review: https://reviews.apache.org/r/50741/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/899ed000
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/899ed000
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/899ed000

Branch: refs/heads/0.27.x
Commit: 899ed000a01ac993dad785b57f3450af7ecb9434
Parents: 329aca4
Author: Greg Mann 
Authored: Fri Aug 5 18:19:33 2016 -0700
Committer: Joseph Wu 
Committed: Thu Sep 15 10:22:18 2016 -0700

--
 3rdparty/libprocess/src/libevent_ssl_socket.cpp | 48 ++--
 1 file changed, 34 insertions(+), 14 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/mesos/blob/899ed000/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 2337e1a..e77781c 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -337,6 +337,12 @@ void LibeventSSLSocketImpl::event_callback(short events)
   // In all of the following conditions, we're interested in swapping
   // the value of the requests with null (if they are already null,
   // then there's no harm).
+  //
+  // TODO(bmahler): If we receive an EOF because the receiving
+  // side only shutdown writes on its socket, we can technically
+  // still send data on the socket!
+  //   See: http://www.unixguide.net/network/socketfaq/2.6.shtml
+  //   Related JIRA: MESOS-5999
   if (events & BEV_EVENT_EOF ||
   events & BEV_EVENT_CONNECTED ||
   events & BEV_EVENT_ERROR) {
@@ -694,14 +700,21 @@ Future LibeventSSLSocketImpl::send(const char* 
data, size_t size)
 CHECK(__in_event_loop__);
 CHECK(self);
 
-// We check that send_request is valid, because we do not
-// allow discards. This means there is no race between the
-// entry of 'send' and the execution of this lambda.
+// Check if the socket is closed or the write end has
+// encountered an error in the interim (i.e. we received
+// a BEV_EVENT_ERROR with BEV_EVENT_WRITING).
+bool write = false;
+
 synchronized (self->lock) {
-  CHECK_NOTNULL(self->send_request.get());
+  if (self->send_request.get() != nullptr) {
+write = true;
+  }
 }
 
-bufferevent_write(self->bev, data, size);
+if (write) {
+  int result = bufferevent_write(self->bev, data, size);
+  CHECK_EQ(0, result);
+}
   },
   DISALLOW_SHORT_CIRCUIT);
 
@@ -738,18 +751,25 @@ Future LibeventSSLSocketImpl::sendfile(
 CHECK(__in_event_loop__);
 CHECK(self);
 
-// We check that send_request is valid, because we do not
-// allow discards. This means there is no race between the
-// entry of 'sendfile' and the execution of this lambda.
+// Check if the socket is closed or the write end has
+// encountered an error in the interim (i.e. we received
+// a BEV_EVENT_ERROR with BEV_EVENT_WRITING).
+bool write = false;
+
 synchronized (self->lock) {
-  CHECK_NOTNULL(self->send_request.get());
+  if (self->send_request.get() != nullptr) {
+write = true;
+  }
 }
 
-evbuffer_add_file(
-bufferevent_get_output(self->bev),
-fd,
-offset,
-size);
+if (write) {
+  int result = evbuffer_add_file(
+  bufferevent_get_output(self->bev),
+  fd,
+  offset,
+  size);
+  CHECK_EQ(0, result);
+}
   },
   DISALLOW_SHORT_CIRCUIT);