[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Vedant Kumar via Phabricator via lldb-commits]

vsk marked an inline comment as done.
vsk added a comment.

In D69148#1715532 <https://reviews.llvm.org/D69148#1715532>, @labath wrote:

> In D69148#1714785 <https://reviews.llvm.org/D69148#1714785>, @vsk wrote:
>
> > The death tests are flaky. I've noticed two issues:
> >
> > 1. When run under lit, the DisableExitOnSIGPIPE doesn't actually exit when it receives SIGPIPE. Dtrace suggests that the unit test process inherits an "ignore" sigaction from its parent.
> > 2. When run in parallel, exiting causes the unit test to run atexit destructors for other unit tests lumped into the SupportTests binary. One of these is a condition_variable destructor and it hangs. Also, gtest warns: `Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 21 threads.`
> >
> >   So I plan to give up on testing "EnableExitOnSIGPIPE" and will write a non-death test to get some coverage.
>
>
> gtest gives you a (somewhat rudimentary) mechanism of ensuring death tests run in a single threaded fashion. It consists of tacking "DeathTest" at the end of your test case name (like `TEST(SignalDeathTest, Whatever)`). These kinds of tests are run first, hopefully before the tests which spin up various other threads.


A problem I'm encountering with this is that the static initializer from Signals.inc don't appear to be re-run between death tests. This makes the death tests pretty brittle, imo, as swapping the order of the tests can break them. Do you think the extra coverage is worth it?

> As for the parent "ignore" action (it's more likely to be a signal mask, actually), this can be reset with an appropriate sigprocmask(2) call.

Thanks, that's a great catch.



================
Comment at: lldb/tools/driver/Driver.cpp:850
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
----------------
labath wrote:
> I don't think this line does anything anymore.
I don't think that's true. LLVM still does `registerHandler(SIGPIPE, SignalKind::IsKill)` in `RegisterHandlers`. At least, removing the `signal(SIGPIPE, SIG_IGN)` calls causes the unit test to "fail" (i.e. the test RUNs but doesn't reach OK).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69148/new/

https://reviews.llvm.org/D69148