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 <g...@mesosphere.io>
Authored: Fri Aug 5 18:19:33 2016 -0700
Committer: Joseph Wu <josep...@apache.org>
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<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;
+          }
         }
 
-        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<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);
 

Reply via email to