----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50477/#review143784 -----------------------------------------------------------
Fix it, then Ship it! LGTM + some comment tweaks. This review is almost a revert of this patch: https://github.com/apache/mesos/commit/ca3667f4e97e11ad30811753fdb52bc02854113f You may want to reference that commit and/or the reasoning behind it. 3rdparty/libprocess/include/process/socket.hpp (line 233) <https://reviews.apache.org/r/50477/#comment209728> s/fd/file descriptor/ To be consistent with other comments in this file. 3rdparty/libprocess/src/libevent_ssl_socket.cpp <https://reviews.apache.org/r/50477/#comment209731> This comment still seems relevant. 3rdparty/libprocess/src/libevent_ssl_socket.cpp (lines 138 - 140) <https://reviews.apache.org/r/50477/#comment209730> This comment is no longer accurate. - Joseph Wu On July 26, 2016, 7:03 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50477/ > ----------------------------------------------------------- > > (Updated July 26, 2016, 7:03 p.m.) > > > Review request for mesos, Joris Van Remoortere and Joseph Wu. > > > Bugs: MESOS-5913 > https://issues.apache.org/jira/browse/MESOS-5913 > > > Repository: mesos > > > Description > ------- > > The deferred call to SSL_shutdown within ~LibeventSSLSocketImpl > can occur after the socket fd has been closed by Socket::~Impl. > > This can lead to a TLS Alert message being sent on any fd if > it the fd is re-used between the close and the SSL_shutdown! > > Thanks to Jan-Philip Gehrcke for reporting the issue. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/socket.hpp > 881b44b987e5894cac838dae046ab7dbad20b000 > 3rdparty/libprocess/src/libevent_ssl_socket.hpp > 4d376d8b7c1b29105de69bed2e4077f8c94fed0b > 3rdparty/libprocess/src/libevent_ssl_socket.cpp > f4c0b0b97960400b0282837979bf0ed17f56a068 > > Diff: https://reviews.apache.org/r/50477/diff/ > > > Testing > ------- > > make check > > Ran my repro steps (issue HTTP requests while hammering the master with HTTPS > requests). Issue disappears after this fix. > > > Thanks, > > Benjamin Mahler > >
