labath added a comment.
> AFAICT kill is entirely asynchronous
This is not exactly true. POSIX describes this situation quite well (emphasis
mine):
> If the value of pid causes sig to be generated for the sending process, and
> if sig is not blocked for the calling thread and if no other thread has sig
> unblocked or is waiting in a sigwait() function for sig, **either sig or at
> least one pending unblocked signal shall be delivered to the sending thread
> before kill() returns**.
Our problem is that this sentence does not apply here, not for one, but two
reasons:
- "sig is not blocked for the calling thread" -- the calling thread in fact has
the signal blocked. That is expected, as it will be unblocked (and delivered)
in the `ppoll` call inside `MainLoop::Run`. That is a pretty good way to catch
signals race-free, but it also relies on the another part of the sentence above.
- "no other thread has sig unblocked" -- it only works if there are no other
threads willing to accept that signal, and I believe that is what is failing us
here. This test does in fact create an extra thread in its SetUp function on
line 35. By the time we leave the SetUp function, that thread has finished with
its useful work (producing the future object), but I suspect what is happening
is that, occasionally, the OS-level thread fails to exit on time and eats our
signal.
In this case, I believe that the simplest way to fix this is to get rid of that
thread. I believe it was necessary at some point in the past (when we were
doing the Listen+Accept calls as a single action), but now it is not necessary
as the self-connection can be completed without having two threads actively
connecting to each other -- it's enough that one socket declares its intent to
accept (listen to) a connection. That will make the test simpler. and I believe
it will also fix the flakyness you observed.
================
Comment at: lldb/unittests/Host/MainLoopTest.cpp:35
Socket *accept_socket;
std::future<Status> accept_error = std::async(std::launch::async, [&] {
return listen_socket_up->Accept(accept_socket);
----------------
Thread created here.
================
Comment at: lldb/unittests/Host/MainLoopTest.cpp:45
ASSERT_TRUE(error.Success());
ASSERT_TRUE(accept_error.get().Success());
----------------
Delete the async call above, and put something like
`ASSERT_TRUE(listen_socket_up->Accept(accept_socket).Success())` here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133181/new/
https://reviews.llvm.org/D133181
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
