Skip to content

Commit 203232f

Browse files
authored
[llvm][Support] ListeningSocket::accept returns operation_canceled if FD is set to -1 (llvm#89479)
If `::poll` returns and `FD` equals -1, then `ListeningSocket::shutdown` has been called. So, regardless of any other information that could be gleaned from `FDs.revents` or `PollStatus`, it is appropriate to return `std::errc::operation_canceled`. `ListeningSocket::shutdown` copies `FD`'s value to `ObservedFD` then sets `FD` to -1 before canceling `::poll` by calling `::close(ObservedFD)` and writing to the pipe.
1 parent 0170bd5 commit 203232f

File tree

2 files changed

+17
-25
lines changed

2 files changed

+17
-25
lines changed

llvm/lib/Support/raw_socket_stream.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -204,31 +204,34 @@ ListeningSocket::accept(std::chrono::milliseconds Timeout) {
204204
auto Start = std::chrono::steady_clock::now();
205205
#ifdef _WIN32
206206
PollStatus = WSAPoll(FDs, 2, RemainingTime);
207-
if (PollStatus == SOCKET_ERROR) {
208207
#else
209208
PollStatus = ::poll(FDs, 2, RemainingTime);
209+
#endif
210+
// If FD equals -1 then ListeningSocket::shutdown has been called and it is
211+
// appropriate to return operation_canceled
212+
if (FD.load() == -1)
213+
return llvm::make_error<StringError>(
214+
std::make_error_code(std::errc::operation_canceled),
215+
"Accept canceled");
216+
217+
#if _WIN32
218+
if (PollStatus == SOCKET_ERROR) {
219+
#else
210220
if (PollStatus == -1) {
211221
#endif
212-
// Ignore error if caused by interupting signal
213222
std::error_code PollErrCode = getLastSocketErrorCode();
223+
// Ignore EINTR (signal occured before any request event) and retry
214224
if (PollErrCode != std::errc::interrupted)
215225
return llvm::make_error<StringError>(PollErrCode, "FD poll failed");
216226
}
217-
218227
if (PollStatus == 0)
219228
return llvm::make_error<StringError>(
220229
std::make_error_code(std::errc::timed_out),
221230
"No client requests within timeout window");
222231

223232
if (FDs[0].revents & POLLNVAL)
224233
return llvm::make_error<StringError>(
225-
std::make_error_code(std::errc::bad_file_descriptor),
226-
"File descriptor closed by another thread");
227-
228-
if (FDs[1].revents & POLLIN)
229-
return llvm::make_error<StringError>(
230-
std::make_error_code(std::errc::operation_canceled),
231-
"Accept canceled");
234+
std::make_error_code(std::errc::bad_file_descriptor));
232235

233236
auto Stop = std::chrono::steady_clock::now();
234237
ElapsedTime +=

llvm/unittests/Support/raw_socket_stream_test.cpp

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
#include "llvm/Testing/Support/Error.h"
88
#include "gtest/gtest.h"
99
#include <future>
10-
#include <iostream>
1110
#include <stdlib.h>
1211
#include <thread>
1312

@@ -86,13 +85,8 @@ TEST(raw_socket_streamTest, TIMEOUT_PROVIDED) {
8685
std::chrono::milliseconds Timeout = std::chrono::milliseconds(100);
8786
Expected<std::unique_ptr<raw_socket_stream>> MaybeServer =
8887
ServerListener.accept(Timeout);
89-
90-
ASSERT_THAT_EXPECTED(MaybeServer, Failed());
91-
llvm::Error Err = MaybeServer.takeError();
92-
llvm::handleAllErrors(std::move(Err), [&](const llvm::StringError &SE) {
93-
std::error_code EC = SE.convertToErrorCode();
94-
ASSERT_EQ(EC, std::errc::timed_out);
95-
});
88+
ASSERT_EQ(llvm::errorToErrorCode(MaybeServer.takeError()),
89+
std::errc::timed_out);
9690
}
9791

9892
TEST(raw_socket_streamTest, FILE_DESCRIPTOR_CLOSED) {
@@ -122,12 +116,7 @@ TEST(raw_socket_streamTest, FILE_DESCRIPTOR_CLOSED) {
122116

123117
// Wait for the CloseThread to finish
124118
CloseThread.join();
125-
126-
ASSERT_THAT_EXPECTED(MaybeServer, Failed());
127-
llvm::Error Err = MaybeServer.takeError();
128-
llvm::handleAllErrors(std::move(Err), [&](const llvm::StringError &SE) {
129-
std::error_code EC = SE.convertToErrorCode();
130-
ASSERT_EQ(EC, std::errc::operation_canceled);
131-
});
119+
ASSERT_EQ(llvm::errorToErrorCode(MaybeServer.takeError()),
120+
std::errc::operation_canceled);
132121
}
133122
} // namespace

0 commit comments

Comments
 (0)