mesos git commit: Libevent: SSL: Changed ownership semantics of ssl connect socket.

[jo...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master 208d7031b -> ca3667f4e


Libevent: SSL: Changed ownership semantics of ssl connect socket.

libprocess Socket shares the ownership of the file descriptor with
libevent. In the destructor of the libprocess libevent_ssl socket, we
call ssl shutdown which is executed asynchronously. This causes the
libprocess socket file descriptor to be closed (and possibly reused)
when the same file descriptor could be used by libevent/ssl. Since we
set the shutdown options as SSL_RECEIVED_SHUTDOWN, we allow write
operations to continue with possibly closed file descriptor.

This change solves the above issue by copying(dup) the original file
descriptor and hands over the copy to libevent ssl. The copied
descriptor is then managed by libprocess Socket.

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


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

Branch: refs/heads/master
Commit: ca3667f4e97e11ad30811753fdb52bc02854113f
Parents: 208d703
Author: Jojy Varghese <jo...@mesosphere.io>
Authored: Fri Dec 18 14:54:03 2015 -0800
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Fri Dec 18 15:56:32 2015 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/src/libevent_ssl_socket.cpp | 32 +++++++++++++++++---
 3rdparty/libprocess/src/libevent_ssl_socket.hpp |  6 ++++
 2 files changed, 34 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ca3667f4/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 55b91dd..a0389a3 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -106,9 +106,10 @@ LibeventSSLSocketImpl::~LibeventSSLSocketImpl()
   evconnlistener* _listener = listener;
   bufferevent* _bev = bev;
   std::weak_ptr<LibeventSSLSocketImpl>* _event_loop_handle = event_loop_handle;
+  int ssl_socket_fd = ssl_connect_fd;
 
   run_in_event_loop(
-      [_listener, _bev, _event_loop_handle]() {
+      [_listener, _bev, _event_loop_handle, ssl_socket_fd]() {
         // Once this lambda is called, it should not be possible for
         // more event loop callbacks to be triggered with 'this->bev'.
         // This is important because we delete event_loop_handle which
@@ -141,6 +142,14 @@ LibeventSSLSocketImpl::~LibeventSSLSocketImpl()
           bufferevent_free(_bev);
         }
 
+        if (ssl_socket_fd >= 0) {
+          Try<Nothing> close = os::close(ssl_socket_fd);
+          if (close.isError()) {
+            LOG(WARNING) << "Failed to close socket "
+                         << stringify(ssl_socket_fd) << ": " << close.error();
+          }
+        }
+
         delete _event_loop_handle;
       },
       DISALLOW_SHORT_CIRCUIT);
@@ -418,7 +427,8 @@ LibeventSSLSocketImpl::LibeventSSLSocketImpl(int _s)
     recv_request(NULL),
     send_request(NULL),
     connect_request(NULL),
-    event_loop_handle(NULL) {}
+    event_loop_handle(NULL),
+    ssl_connect_fd(-1) {}
 
 
 LibeventSSLSocketImpl::LibeventSSLSocketImpl(
@@ -433,7 +443,8 @@ LibeventSSLSocketImpl::LibeventSSLSocketImpl(
     send_request(NULL),
     connect_request(NULL),
     event_loop_handle(NULL),
-    peer_hostname(std::move(_peer_hostname)) {}
+    peer_hostname(std::move(_peer_hostname)),
+    ssl_connect_fd(-1) {}
 
 
 Future<Nothing> LibeventSSLSocketImpl::connect(const Address& address)
@@ -451,13 +462,26 @@ Future<Nothing> LibeventSSLSocketImpl::connect(const Address& address)
     return Failure("Failed to connect: SSL_new");
   }
 
+  ssl_connect_fd = ::dup(get());
+  if (ssl_connect_fd < 0) {
+    return Failure("Failed to 'dup' socket for new openssl socket handle");
+  }
+
+  // Reapply FD_CLOEXEC on the duplicate file descriptor.
+  Try<Nothing> closeexec = os::cloexec(ssl_connect_fd);
+  if (closeexec.isError()) {
+    return Failure(
+        "Failed to set FD_CLOEXEC flag for the dup'ed openssl socket handle: " +
+        closeexec.error());
+  }
+
   // Construct the bufferevent in the connecting state.
   // We set 'BEV_OPT_DEFER_CALLBACKS' to avoid calling the
   // 'event_callback' before 'bufferevent_socket_connect' returns.
   CHECK(bev == NULL);
   bev = bufferevent_openssl_socket_new(
       base,
-      get(),
+      ssl_connect_fd,
       ssl,
       BUFFEREVENT_SSL_CONNECTING,
       BEV_OPT_THREADSAFE | BEV_OPT_DEFER_CALLBACKS);

http://git-wip-us.apache.org/repos/asf/mesos/blob/ca3667f4/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 2669b1a..e773fad 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.hpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.hpp
@@ -178,6 +178,12 @@ private:
   Queue<Future<Socket>> accept_queue;
 
   Option<std::string> peer_hostname;
+
+  // Socket descriptor/handle used by libevent_ssl.
+  // Ownership semantics:
+  //  This class owns this handle and is responsible for creating (via dup)
+  //  and closing it.
+  int ssl_connect_fd;
 };
 
 } // namespace network {