----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65183/#review196174 -----------------------------------------------------------
Ship it! LGTM. 3rdparty/libprocess/src/process.cpp Lines 893-899 (original), 901-910 (patched) <https://reviews.apache.org/r/65183/#comment275707> Checking `__s__ != nullptr` here no longer serves any purpose, as the `Future<Socket>& socket` will always be discarded before the `__s__` is deleted. Perhaps change it to: ``` if (!stopped) { synchronized (socket_mutex) { CHECK_NOTNULL(__s__); future_accept = ... } } ``` - Joseph Wu On Jan. 24, 2018, 12:47 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65183/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2018, 12:47 p.m.) > > > Review request for mesos, Benjamin Hindman, Joseph Wu, and Michael Park. > > > Repository: mesos > > > Description > ------- > > Once discard support for Queue::get was introduced, this makes it > possible for the finalization logic in libprocess to try to acquire > the `socket_mutex` recursively: > > (1) process::finalize acquires socket_mutex, and discards __s__. > (2) The get from LibeventSSLSocket::accept_queue gets discarded. > (3) This then executes the process.cpp logic in on_accept, which > will always re-aquire the `socket_mutex`, even though the > accepted socket came out as discarded. > > The fix applied here is to also stop the accept loop when the socket > was discarded. This avoids acquiring the `socket_mutex` in the > discarded case. > > Also, this avoids logging a failure message in the discarded case, > and instead consistently logs when the accept loop is stopped. > > Also, wrote a comment within `LibeventSSLSocket::accept` that > explains a potential issue due to MESOS-8448. > > > Diffs > ----- > > 3rdparty/libprocess/src/libevent_ssl_socket.cpp > 1c95ebabfefd07aaeb053b965ab8e4550dfccaef > 3rdparty/libprocess/src/process.cpp > 2126c272a69bfa53b4b3cbbb1a55a3f81a5da8ad > > > Diff: https://reviews.apache.org/r/65183/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Mahler > >
