----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50936/#review145284 -----------------------------------------------------------
Thanks Greg! 3rdparty/libprocess/src/poll_socket.cpp (line 166) <https://reviews.apache.org/r/50936/#comment211446> Can we avoid passing the int and just get the int from the Socket::Impl? (If we had followed the pattern of pulling the int out of the Socket::Impl we wouldn't have had this lifetime issue :)). 3rdparty/libprocess/src/poll_socket.cpp (lines 259 - 260) <https://reviews.apache.org/r/50936/#comment211445> Don't you need to handle the sendfile path as well? - Benjamin Mahler On Aug. 9, 2016, 9:56 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50936/ > ----------------------------------------------------------- > > (Updated Aug. 9, 2016, 9:56 p.m.) > > > Review request for mesos, Benjamin Mahler, Joseph Wu, and Vinod Kone. > > > Bugs: MESOS-5988 > https://issues.apache.org/jira/browse/MESOS-5988 > > > Repository: mesos > > > Description > ------- > > Currently, the poll socket implementation doesn't keep > the socket alive after `PollSocketImpl.send(data, length)` > passes execution off to a continuation. This patch passes > a shared pointer of the socket implementation along to the > continuation, keeping the socket alive. > > > Diffs > ----- > > 3rdparty/libprocess/src/poll_socket.cpp > d14dd1ae210e112b917329803dfb09d98386b966 > > Diff: https://reviews.apache.org/r/50936/diff/ > > > Testing > ------- > > I was able to expose the poll socket bug with the following test case. It > triggers a "Bad file descriptor" error after 100-200 repetitions on my OSX > machine. After applying this patch, I can run the test thousands of times > with no failures: > > ``` > TEST(HTTPTest, ClosedSocketFailure) > { > const Try<Socket> server_create = Socket::create(); > ASSERT_SOME(server_create); > > Socket server = server_create.get(); > LOG(INFO) << "*** server fd: " << server.get(); > > Future<size_t> sent; > Future<Socket> _socket; > > string data; > for (auto i = 0; i < 26000; i++) { > data += > "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012"; > } > const char* data_cstr = data.c_str(); > > { > const Try<Socket> client_create = Socket::create(); > ASSERT_SOME(client_create); > > Socket client = client_create.get(); > LOG(INFO) << "*** client fd: " << client.get(); > > const Try<Nothing> listen = server.listen(5); > ASSERT_SOME(listen); > > const Try<Address> server_address = server.address(); > ASSERT_SOME(server_address); > > _socket = server.accept(); > > const Future<Nothing> connect = client.connect(server_address.get()); > > // Wait for the server to have accepted the client connection. > AWAIT_ASSERT_READY(_socket); > Socket socket = _socket.get(); // TODO(jmlvanre): Remove const copy. > > // Verify that the client also views the connection as established. > AWAIT_ASSERT_READY(connect); > > // Send a message from the client to the server. > sent = client.send(data_cstr, data.size()); > LOG(INFO) << "*** exiting scope"; > } > > AWAIT_READY(sent); > > LOG(INFO) << "*** exiting test case"; > } > ``` > > > Thanks, > > Greg Mann > >
