mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jo...@apache.org
Subject mesos git commit: Libevent SSL: Added check for buffer length before swapping request.
Date Tue, 22 Dec 2015 20:54:38 GMT
Repository: mesos
Updated Branches:
  refs/heads/master 92e8aee58 -> 0461aaba6


Libevent SSL: Added check for buffer length before swapping request.

recv_callback could be called from libevent's receive callback and
Socket::recv for the same buffer event and different requests. There
is a check for buffer length at Socket::recv but not at libevent's
receive callback. This could lead to the incoming request for
Socket::recv being swapped out even though the buffer length is zero.
This change adds a check for buffer length before swapping out the
receive request object.

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


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

Branch: refs/heads/master
Commit: 0461aaba695a33ef8e92f3818e217dd23926cb50
Parents: 92e8aee
Author: Jojy Varghese <jojy@mesosphere.io>
Authored: Tue Dec 22 15:09:46 2015 -0500
Committer: Joris Van Remoortere <joris.van.remoortere@gmail.com>
Committed: Tue Dec 22 15:53:02 2015 -0500

----------------------------------------------------------------------
 3rdparty/libprocess/src/libevent_ssl_socket.cpp | 27 +++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0461aaba/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 a0389a3..9321ce6 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -232,15 +232,36 @@ void LibeventSSLSocketImpl::recv_callback(bufferevent* /*bev*/, void*
arg)
 
 
 // Only runs in event loop. Member function continuation of static
-// 'recv_callback'.
+// 'recv_callback'. This function can be called from two places -
+// a) `LibeventSSLSocketImpl::recv` when a new Socket::recv is called and there
+//    is buffer available to read.
+// b) `LibeventSSLSocketImpl::recv_callback when libevent calls the deferred
+//    read callback.
 void LibeventSSLSocketImpl::recv_callback()
 {
   CHECK(__in_event_loop__);
 
   Owned<RecvRequest> request;
 
-  synchronized (lock) {
-    std::swap(request, recv_request);
+  const size_t buffer_length = evbuffer_get_length(bufferevent_get_input(bev));
+
+  // Swap out the request object IFF there is buffer available to read. We check
+  // this here because it is possible that when the libevent deferred callback
+  // was called, a Socket::recv context already read the buffer from the event.
+  // Following sequence is possible:
+  // a. libevent finds a buffer ready to be read.
+  // b. libevent queues buffer event to be dispatched.
+  // c. Socket::recv is called that creates a new request.
+  // d. Socket::recv finds buffer length > 0.
+  // e. Socket::recv reads the buffer.
+  // f. A new request Socket::recv is called which creates a new request.
+  // g. libevent callback is called for the event queued at step b.
+  // h. libevent callback finds the length of the buffer as 0 but the request is
+  //    a non-NULL due to step f.
+  if (buffer_length > 0) {
+    synchronized (lock) {
+      std::swap(request, recv_request);
+    }
   }
 
   if (request.get() != NULL) {


Mime
View raw message