----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49280/#review139683 -----------------------------------------------------------
3rdparty/libprocess/src/libevent_ssl_socket.cpp (lines 808 - 810) <https://reviews.apache.org/r/49280/#comment204985> I would have expected this note to appear above `evconnlistener_new`. Here I would expect a note explaining that a library implementation should not be opinionated about `cloexec`, and rather delegate that decision to the user. Take the linux socket as an example: http://man7.org/linux/man-pages/man2/socket.2.html We should either: 1) parameterize the socket interface to expose this option, and maybe provide the default that is most commonly used by libprocess. On the flip side you should argue that the default should be the one of least surprise, which would be *not* to provide `cloexec` in line with the linux API. 2) add a `TODO` here that explains this predicament, and point to the JIRA to take on this cleanup work. I propose #2 as we're trying to fix a bug before a release, and may not spend sufficient time to think through #1 correctly. This also seems like something that would be a valuable contribution by a community member. The same would apply for the `connect` code at line 489 3rdparty/libprocess/src/libevent_ssl_socket.cpp (lines 813 - 814) <https://reviews.apache.org/r/49280/#comment204998> Is this behavior consistent with the other implementations? If an error occurs, we don't propagate it to the `acceptor`? I think a comment is necessary here to explain what behavior we're accomplishing. 3rdparty/libprocess/src/libevent_ssl_socket.cpp (line 837) <https://reviews.apache.org/r/49280/#comment204991> Since we're trying to consistently use `cloexec` in this implementation in the interim, why do we not need to use `LEV_OPT_CLOSE_ON_EXEC` here? - Joris Van Remoortere On June 27, 2016, 7:38 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49280/ > ----------------------------------------------------------- > > (Updated June 27, 2016, 7:38 p.m.) > > > Review request for mesos, Avinash sridharan, Benjamin Mahler, Artem > Harutyunyan, and Joris Van Remoortere. > > > Bugs: MESOS-5723 > https://issues.apache.org/jira/browse/MESOS-5723 > > > Repository: mesos > > > Description > ------- > > Incoming sockets are leaked when the agent forks because incoming > sockets are not modified with the CLOEXEC option. > > > Diffs > ----- > > 3rdparty/libprocess/src/libevent_ssl_socket.cpp > 3f80e863fdbb62d4cf7ce1b46c468a09f0694f56 > > Diff: https://reviews.apache.org/r/49280/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Joseph Wu > >
