> On Dec. 9, 2016, 1 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/libevent_ssl_socket.cpp, lines 366-367 > > <https://reviews.apache.org/r/53802/diff/6/?file=1573360#file1573360line366> > > > > Let's remove this part of the case: > > > > ``` > > events & BEV_EVENT_ERROR && EVUTIL_SOCKET_ERROR() == 0) > > ``` > > > > and take the CHECK out of the final error case. We can just call > > evutil_socket_error_to_string even if it's 0 AFAICT (although you should > > check on windows if it's ok). > > Greg Mann wrote: > I checked out the windows code in libevent and calling > `evutil_socket_error_to_string(0)` looks safe: > > ``` > const char * > evutil_socket_error_to_string(int errcode) > { > /* XXXX Is there really no built-in function to do this? */ > int i; > for (i=0; windows_socket_errors[i].code >= 0; ++i) { > if (errcode == windows_socket_errors[i].code) > return windows_socket_errors[i].msg; > } > return strerror(errcode); > } > ```
After removing the extra conditional, my new SocketEOF tests are throwing an error because they are reaching the error-handling code in the event callback, instead of the EOF-handling code. It would seem that checking for `events & BEV_EVENT_ERROR && EVUTIL_SOCKET_ERROR() == 0` is necessary to detect EOF, but I'm not sure why :( - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53802/#review158604 ----------------------------------------------------------- On Dec. 9, 2016, 9:19 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53802/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2016, 9:19 p.m.) > > > Review request for mesos, Benjamin Mahler and Joseph Wu. > > > Bugs: MESOS-5966 > https://issues.apache.org/jira/browse/MESOS-5966 > > > Repository: mesos > > > Description > ------- > > Previously, it was possible for an SSL socket to either: > 1) Fail to receive an EOF if the EOF event was received when > there was no pending recv() request. > 2) Fail to receive all data sent on the sending side if an > EOF event was received before all sent data was read. > > This patch eliminates these race conditions to ensure reliable > receipt of both sent data and EOFs. > > > Diffs > ----- > > 3rdparty/libprocess/src/libevent_ssl_socket.hpp > 57eaf4f607d0628db466cc1a139772eeeaa51136 > 3rdparty/libprocess/src/libevent_ssl_socket.cpp > dddd0e292a8b0d470f4e199db08f09a0c863d73c > > Diff: https://reviews.apache.org/r/53802/diff/ > > > Testing > ------- > > Testing details are found at the end of this chain. > > > Thanks, > > Greg Mann > >
