[3/5] mesos git commit: Fixed potential FD double close in the libevent socket.

[jo...@apache.org]

Fixed potential FD double close in the libevent socket.

`evbuffer_add_file` will take ownership of the file descriptor passed
into it.  Normally, this file descriptor is owned by the `FileEncoder`
in the libprocess's `SocketManager`.  Since there are two owners, one
of the owners may close the file descriptor when it has been re-used.

In this case, when multiple threads access the master's web UI at once
with SSL enabled, the master may CHECK-fail due to a bad (closed)
file descriptor.

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


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

Branch: refs/heads/0.28.x
Commit: 96d73c7874e850794527c9d07f3aa9e0de7d4585
Parents: 237f316
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Mon Sep 12 12:40:09 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Sep 15 10:18:44 2016 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/src/libevent_ssl_socket.cpp | 38 ++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/96d73c78/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 77af7b3..c0fd1d6 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -737,6 +737,36 @@ Future<size_t> LibeventSSLSocketImpl::sendfile(
     std::swap(request, send_request);
   }
 
+  // Duplicate the file descriptor because Libevent will take ownership
+  // and control the lifecycle separately.
+  //
+  // TODO(josephw): We can avoid duplicating the file descriptor in
+  // future versions of Libevent. In Libevent versions 2.1.2 and later,
+  // we may use `evbuffer_file_segment_new` and `evbuffer_add_file_segment`
+  // instead of `evbuffer_add_file`.
+  int owned_fd = dup(fd);
+  if (fd == -1) {
+    return Failure(ErrnoError("Failed to duplicate file descriptor"));
+  }
+
+  // Set the close-on-exec flag.
+  Try<Nothing> cloexec = os::cloexec(owned_fd);
+  if (cloexec.isError()) {
+    os::close(owned_fd);
+    return Failure(
+        "Failed to set close-on-exec on duplicated file descriptor: " +
+        cloexec.error());
+  }
+
+  // Make the file descriptor non-blocking.
+  Try<Nothing> nonblock = os::nonblock(owned_fd);
+  if (nonblock.isError()) {
+    os::close(owned_fd);
+    return Failure(
+        "Failed to make duplicated file descriptor non-blocking: " +
+        nonblock.error());
+  }
+
   // Extend the life-time of 'this' through the execution of the
   // lambda in the event loop. Note: The 'self' needs to be explicitly
   // captured because we're not using it in the body of the lambda. We
@@ -745,7 +775,7 @@ Future<size_t> LibeventSSLSocketImpl::sendfile(
   auto self = shared(this);
 
   run_in_event_loop(
-      [self, fd, offset, size]() {
+      [self, owned_fd, offset, size]() {
         CHECK(__in_event_loop__);
         CHECK(self);
 
@@ -761,12 +791,16 @@ Future<size_t> LibeventSSLSocketImpl::sendfile(
         }
 
         if (write) {
+          // NOTE: `evbuffer_add_file` will take ownership of the file
+          // descriptor and close it after it has finished reading it.
           int result = evbuffer_add_file(
               bufferevent_get_output(self->bev),
-              fd,
+              owned_fd,
               offset,
               size);
           CHECK_EQ(0, result);
+        } else {
+          os::close(owned_fd);
         }
       },
       DISALLOW_SHORT_CIRCUIT);