[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I actually enjoy debugging things like this, so I tried playing around and came up with this test case:

  // Test that a signal which is not monitored by the MainLoop does not cause a premature exit.
  TEST_F(MainLoopTest, UnmonitoredSignal) {
    MainLoop loop;
    Status error;
    struct sigaction sa;
    sa.sa_sigaction = [](int, siginfo_t *, void *) { };
    sa.sa_flags = SA_SIGINFO; // important: no SA_RESTART
    sigemptyset(&sa.sa_mask);
    ASSERT_EQ(0, sigaction(SIGUSR2, &sa, nullptr));
  
    auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
    ASSERT_TRUE(error.Success());
    std::thread killer([]() {
      sleep(1);
      kill(getpid(), SIGUSR2);
      sleep(1);
      kill(getpid(), SIGUSR1);
    });
    ASSERT_TRUE(loop.Run().Success());
    killer.join();
    ASSERT_EQ(1u, callback_count);
  }

It's not the nicest of tests: it uses sleep, which is necessary to make sure the main thread gets a chance to block in kevent(2), and one needs to be very careful when doing signal catching in a multithreaded environment (e.g. creating the killer thread before registering the SIGUSR1 handler would make the test flaky on linux, as the new thread would not inherit the signal mask which has USR1 blocked). However, I think it should be sufficient for the purposes of this patch.

In case anyone is interested, the fact that attaching a debugger produces an observable side-effect (EINTR) in the debugged process is considered a bug by the linux kernel folks and they have worked hard to fix it. Right now linux kernel will automatically restart most syscalls after a debugger resume regardless of the value of SA_RESTART flags or anything like that. There is just a handful of syscalls that are really complex and they haven't figured out how to resume them part-way. These still return EINTR.



================
Comment at: source/Host/common/MainLoop.cpp:108-112
+  do {
+    errno = 0;
+    num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
+                        out_events, llvm::array_lengthof(out_events), nullptr);
+  } while (num_events == -1 && errno == EINTR);
----------------
There's an `llvm::sys::RetryAfterSignal` which implements the EINTR loop. I had to pass `out_events` as `&out_events[0]` to make template deduction happy.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42206