Skip to content

[lldb] [debugserver] Handle interrupted reads correctly #84872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

jasonmolenda
Copy link
Collaborator

The first half of this patch is a long-standing annoyance, if I attach to debugserver with lldb while it is waiting for an lldb connection, the syscall is interrupted and it doesn't retry, debugserver exits immediately.

The second half is a request from another tool that is communicating with debugserver, that we retry reads on our sockets in the same way. I haven't dug in to the details of how they're communicating that this is necessary, but the best I've been able to find reading the POSIX API docs, this is fine.

rdar://117113298

The first half of this patch is a long-standing annoyance,
if I attach to debugserver with lldb while it is waiting
for an lldb connection, the syscall is interrupted and it
doesn't retry, debugserver exits immediately.

The second half is a request from another tool that is
communicating with debugserver, that we retry reads on
our sockets in the same way.  I haven't dug in to the
details of how they're communicating that this is necessary,
but the best I've been able to find reading the POSIX API
docs, this is fine.

rdar://117113298
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2024

@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)

Changes

The first half of this patch is a long-standing annoyance, if I attach to debugserver with lldb while it is waiting for an lldb connection, the syscall is interrupted and it doesn't retry, debugserver exits immediately.

The second half is a request from another tool that is communicating with debugserver, that we retry reads on our sockets in the same way. I haven't dug in to the details of how they're communicating that this is necessary, but the best I've been able to find reading the POSIX API docs, this is fine.

rdar://117113298


Full diff: https://github.com/llvm/llvm-project/pull/84872.diff

1 Files Affected:

  • (modified) lldb/tools/debugserver/source/RNBSocket.cpp (+13-3)
diff --git a/lldb/tools/debugserver/source/RNBSocket.cpp b/lldb/tools/debugserver/source/RNBSocket.cpp
index 1282ea221625a0..fc55dbf2e1f1b0 100644
--- a/lldb/tools/debugserver/source/RNBSocket.cpp
+++ b/lldb/tools/debugserver/source/RNBSocket.cpp
@@ -120,8 +120,13 @@ rnb_err_t RNBSocket::Listen(const char *listen_host, uint16_t port,
   while (!accept_connection) {
 
     struct kevent event_list[4];
-    int num_events =
-        kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+    int num_events;
+    do {
+      errno = 0;
+      num_events =
+          kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+    } while (num_events == -1 &&
+             (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
 
     if (num_events < 0) {
       err.SetError(errno, DNBError::MachKernel);
@@ -291,7 +296,12 @@ rnb_err_t RNBSocket::Read(std::string &p) {
   // DNBLogThreadedIf(LOG_RNB_COMM, "%8u RNBSocket::%s calling read()",
   // (uint32_t)m_timer.ElapsedMicroSeconds(true), __FUNCTION__);
   DNBError err;
-  ssize_t bytesread = read(m_fd, buf, sizeof(buf));
+  ssize_t bytesread;
+  do {
+    errno = 0;
+    bytesread = read(m_fd, buf, sizeof(buf));
+  } while (bytesread == -1 &&
+           (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
   if (bytesread <= 0)
     err.SetError(errno, DNBError::POSIX);
   else

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. LGTM.

@jasonmolenda jasonmolenda merged commit 87dc068 into llvm:main Mar 12, 2024
@jasonmolenda jasonmolenda deleted the debugserver-syscall-retries branch March 12, 2024 16:03
jasonmolenda added a commit to jasonmolenda/llvm-project that referenced this pull request Mar 12, 2024
The first half of this patch is a long-standing annoyance, if I attach
to debugserver with lldb while it is waiting for an lldb connection, the
syscall is interrupted and it doesn't retry, debugserver exits
immediately.

The second half is a request from another tool that is communicating
with debugserver, that we retry reads on our sockets in the same way. I
haven't dug in to the details of how they're communicating that this is
necessary, but the best I've been able to find reading the POSIX API
docs, this is fine.

rdar://117113298
(cherry picked from commit 87dc068)
jasonmolenda added a commit to swiftlang/llvm-project that referenced this pull request Mar 13, 2024
…-syscalls

[lldb] [debugserver] Handle interrupted reads correctly (llvm#84872)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants