You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2016/08/08 18:33:55 UTC

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

Repository: mesos
Updated Branches:
  refs/heads/1.0.x 48d728873 -> ea5151b2c


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

Branch: refs/heads/1.0.x
Commit: b1dab04e00b02bf7a65e93bc74d66aeb0ca8d28a
Parents: 48d7288
Author: Greg Mann <gr...@mesosphere.io>
Authored: Fri Aug 5 18:19:33 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Aug 8 11:31:05 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/b1dab04e/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 9212b19..57a63f6 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) {
@@ -696,14 +702,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);
 
@@ -740,18 +753,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);
 


[2/2] mesos git commit: Added MESOS-5986 to the 1.0.1 CHANGELOG.

Posted by bm...@apache.org.
Added MESOS-5986 to the 1.0.1 CHANGELOG.


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

Branch: refs/heads/1.0.x
Commit: ea5151b2ce20d0ae1ed794a538bc2fe7e7ef4732
Parents: b1dab04
Author: Benjamin Mahler <bm...@apache.org>
Authored: Mon Aug 8 11:32:23 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Aug 8 11:32:23 2016 -0700

----------------------------------------------------------------------
 CHANGELOG | 2 ++
 1 file changed, 2 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ea5151b2/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 0191a09..2870d19 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -16,6 +16,8 @@ All Issues:
   * [MESOS-5945] - NvidiaVolume::create() should check for root before creating volume.
   * [MESOS-5959] - All non-root tests fail on GPU machine.
   * [MESOS-5982] - NvidiaVolume errors out if any Nvidia binary is missing.
+  * [MESOS-5986] - SSL Socket CHECK can fail after socket receives EOF.
+
 
 Release Notes - Mesos - Version 1.0.0
 --------------------------------------------