mesos git commit: Removed incorrect CHECK in SSL socket `send()`.

[bm...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master 50f591a78 -> f5822f3c1


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/f5822f3c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f5822f3c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f5822f3c

Branch: refs/heads/master
Commit: f5822f3c13f4fdacbb390341940d3379248a9837
Parents: 50f591a
Author: Greg Mann <gr...@mesosphere.io>
Authored: Fri Aug 5 18:19:33 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri Aug 5 18:38:57 2016 -0700

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


http://git-wip-us.apache.org/repos/asf/mesos/blob/f5822f3c/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 97af3c2..05c2bd4 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) {
@@ -701,15 +707,21 @@ Future<size_t> 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;
+          }
         }
 
-        int result = bufferevent_write_buffer(self->bev, buffer);
-        CHECK_EQ(0, result);
+        if (write) {
+          int result = bufferevent_write_buffer(self->bev, buffer);
+          CHECK_EQ(0, result);
+        }
 
         evbuffer_free(buffer);
       },
@@ -748,18 +760,25 @@ Future<size_t> 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);