mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joris Van Remoortere" <joris.van.remoort...@gmail.com>
Subject Re: Review Request 41026: libevent ssl: Added check for buffer length before swapping request.
Date Fri, 18 Dec 2015 22:48:46 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41026/#review111295
-----------------------------------------------------------



3rdparty/libprocess/src/libevent_ssl_socket.cpp (lines 225 - 226)
<https://reviews.apache.org/r/41026/#comment171412>

    Let's explain the call-graph entry points here.



3rdparty/libprocess/src/libevent_ssl_socket.cpp (line 233)
<https://reviews.apache.org/r/41026/#comment171411>

    We don't need this as both call paths lock the bev already.
    Let's add a comment at the top of this function to elaborate on both call graphs, and
how they lock the bev before entering.



3rdparty/libprocess/src/libevent_ssl_socket.cpp (line 234)
<https://reviews.apache.org/r/41026/#comment171404>

    We should either do this consistently or just use `size_t`. I vote for `size_t` for now.



3rdparty/libprocess/src/libevent_ssl_socket.cpp (line 237)
<https://reviews.apache.org/r/41026/#comment171413>

    Let's clearly explain why we need this (as per the example event orders we used to discover
this bug).
    We may enter this function scheduled by libevent when it thought the buffer had data,
but between libevent scheduling it and it being invoked, our lambda in `recv()` got invoked
instead.


- Joris Van Remoortere


On Dec. 8, 2015, 1:22 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41026/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 1:22 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4069
>     https://issues.apache.org/jira/browse/MESOS-4069
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> recv_callback could be called from libevents 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.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 55b91dd47bb5bd5e97147d0af91c7899fd42702c

> 
> Diff: https://reviews.apache.org/r/41026/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message