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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to