[llvm] [llvm][Support] ListeningSocket::accept returns operation_canceled if FD is set to -1 (PR #89479)
Connor Sughrue via llvm-commits
llvm-commits at lists.llvm.org
Wed May 8 19:46:16 PDT 2024
https://github.com/cpsughrue updated https://github.com/llvm/llvm-project/pull/89479
>From 1748af4e569fadfb7bc0d0ed93efec0fc96a845c Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Fri, 19 Apr 2024 21:54:42 -0400
Subject: [PATCH 1/4] [llvm][Support] Correct error handling in
ListeningSocket::support to return operation canceled if FD equals -1
---
llvm/lib/Support/raw_socket_stream.cpp | 27 +++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Support/raw_socket_stream.cpp b/llvm/lib/Support/raw_socket_stream.cpp
index 14e2308df4d7e..ff14b3b6e09c1 100644
--- a/llvm/lib/Support/raw_socket_stream.cpp
+++ b/llvm/lib/Support/raw_socket_stream.cpp
@@ -200,21 +200,32 @@ ListeningSocket::accept(std::chrono::milliseconds Timeout) {
while (PollStatus == -1 && (Timeout.count() == -1 || ElapsedTime < Timeout)) {
if (Timeout.count() != -1)
RemainingTime -= ElapsedTime.count();
-
auto Start = std::chrono::steady_clock::now();
+
#ifdef _WIN32
PollStatus = WSAPoll(FDs, 2, RemainingTime);
- if (PollStatus == SOCKET_ERROR) {
#else
PollStatus = ::poll(FDs, 2, RemainingTime);
+#endif
+ // If FD equals -1 then ListeningSocket::shutdown has been called and it is
+ // appropriate to return operation_canceled. ListeningSocket::shutdown
+ // copies FD's value to ObservedFD then sets FD to -1 before canceling
+ // ::poll by calling close on ObservedFD and writing to the pipe.
+ if (FD.load() == -1)
+ return llvm::make_error<StringError>(
+ std::make_error_code(std::errc::operation_canceled),
+ "Accept canceled");
+
+#if _WIN32
+ if (PollStatus == SOCKET_ERROR) {
+#else
if (PollStatus == -1) {
#endif
- // Ignore error if caused by interupting signal
std::error_code PollErrCode = getLastSocketErrorCode();
+ // Ignore EINTR (signal occured before any request event) and retry
if (PollErrCode != std::errc::interrupted)
return llvm::make_error<StringError>(PollErrCode, "FD poll failed");
}
-
if (PollStatus == 0)
return llvm::make_error<StringError>(
std::make_error_code(std::errc::timed_out),
@@ -222,13 +233,7 @@ ListeningSocket::accept(std::chrono::milliseconds Timeout) {
if (FDs[0].revents & POLLNVAL)
return llvm::make_error<StringError>(
- std::make_error_code(std::errc::bad_file_descriptor),
- "File descriptor closed by another thread");
-
- if (FDs[1].revents & POLLIN)
- return llvm::make_error<StringError>(
- std::make_error_code(std::errc::operation_canceled),
- "Accept canceled");
+ std::make_error_code(std::errc::bad_file_descriptor));
auto Stop = std::chrono::steady_clock::now();
ElapsedTime +=
>From 241e2204086dae46099f1235862a74d8711ee5fa Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Thu, 2 May 2024 12:45:36 -0400
Subject: [PATCH 2/4] Address PR Feedback. Whitespace change and comment length
---
llvm/lib/Support/raw_socket_stream.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Support/raw_socket_stream.cpp b/llvm/lib/Support/raw_socket_stream.cpp
index ff14b3b6e09c1..549d537709bf2 100644
--- a/llvm/lib/Support/raw_socket_stream.cpp
+++ b/llvm/lib/Support/raw_socket_stream.cpp
@@ -200,17 +200,15 @@ ListeningSocket::accept(std::chrono::milliseconds Timeout) {
while (PollStatus == -1 && (Timeout.count() == -1 || ElapsedTime < Timeout)) {
if (Timeout.count() != -1)
RemainingTime -= ElapsedTime.count();
- auto Start = std::chrono::steady_clock::now();
+ auto Start = std::chrono::steady_clock::now();
#ifdef _WIN32
PollStatus = WSAPoll(FDs, 2, RemainingTime);
#else
PollStatus = ::poll(FDs, 2, RemainingTime);
#endif
// If FD equals -1 then ListeningSocket::shutdown has been called and it is
- // appropriate to return operation_canceled. ListeningSocket::shutdown
- // copies FD's value to ObservedFD then sets FD to -1 before canceling
- // ::poll by calling close on ObservedFD and writing to the pipe.
+ // appropriate to return operation_canceled
if (FD.load() == -1)
return llvm::make_error<StringError>(
std::make_error_code(std::errc::operation_canceled),
>From bb5b9d3027beb38eb54cb9a3892d71f7ab455b2a Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Thu, 2 May 2024 13:19:40 -0400
Subject: [PATCH 3/4] Remove iostream
---
llvm/unittests/Support/raw_socket_stream_test.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/unittests/Support/raw_socket_stream_test.cpp b/llvm/unittests/Support/raw_socket_stream_test.cpp
index a8536228666db..1786dea87549a 100644
--- a/llvm/unittests/Support/raw_socket_stream_test.cpp
+++ b/llvm/unittests/Support/raw_socket_stream_test.cpp
@@ -7,7 +7,6 @@
#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
#include <future>
-#include <iostream>
#include <stdlib.h>
#include <thread>
>From b24feb88842321444195f8ff21bebc7e44e3c7bb Mon Sep 17 00:00:00 2001
From: cpsughrue <cpsughrue at gmail.com>
Date: Wed, 8 May 2024 22:43:45 -0400
Subject: [PATCH 4/4] Remove ASSERT_THAT_EXPECTED so that takeError is not
called twice on MaybeServer
---
.../Support/raw_socket_stream_test.cpp | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/llvm/unittests/Support/raw_socket_stream_test.cpp b/llvm/unittests/Support/raw_socket_stream_test.cpp
index 1786dea87549a..c4e8cfbbe7e6a 100644
--- a/llvm/unittests/Support/raw_socket_stream_test.cpp
+++ b/llvm/unittests/Support/raw_socket_stream_test.cpp
@@ -85,13 +85,8 @@ TEST(raw_socket_streamTest, TIMEOUT_PROVIDED) {
std::chrono::milliseconds Timeout = std::chrono::milliseconds(100);
Expected<std::unique_ptr<raw_socket_stream>> MaybeServer =
ServerListener.accept(Timeout);
-
- ASSERT_THAT_EXPECTED(MaybeServer, Failed());
- llvm::Error Err = MaybeServer.takeError();
- llvm::handleAllErrors(std::move(Err), [&](const llvm::StringError &SE) {
- std::error_code EC = SE.convertToErrorCode();
- ASSERT_EQ(EC, std::errc::timed_out);
- });
+ ASSERT_EQ(llvm::errorToErrorCode(MaybeServer.takeError()),
+ std::errc::timed_out);
}
TEST(raw_socket_streamTest, FILE_DESCRIPTOR_CLOSED) {
@@ -121,12 +116,7 @@ TEST(raw_socket_streamTest, FILE_DESCRIPTOR_CLOSED) {
// Wait for the CloseThread to finish
CloseThread.join();
-
- ASSERT_THAT_EXPECTED(MaybeServer, Failed());
- llvm::Error Err = MaybeServer.takeError();
- llvm::handleAllErrors(std::move(Err), [&](const llvm::StringError &SE) {
- std::error_code EC = SE.convertToErrorCode();
- ASSERT_EQ(EC, std::errc::operation_canceled);
- });
+ ASSERT_EQ(llvm::errorToErrorCode(MaybeServer.takeError()),
+ std::errc::operation_canceled);
}
} // namespace
More information about the llvm-commits
mailing list