[PATCH] D46858: Signal handling should be signal-safe

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In https://reviews.llvm.org/D46858#1098875, @jfb wrote:

> I committed the two requests NFC changes. Will address other changes tomorrow.


SGTM.

> 
> 
> In https://reviews.llvm.org/D46858#1098873, @dexonsmith wrote:
> 
>> In https://reviews.llvm.org/D46858#1098865, @jfb wrote:
>>
>> > (Will address code changes separately)
>> >
>> > In https://reviews.llvm.org/D46858#1098806, @dexonsmith wrote:
>> >
>> > > I wonder if there's some way to write a test for this.  E.g., a unit test that registers a signal handler for SIGUSR1 and then does what used to be necessary to trigger a crash.
>> >
>> >
>> > Hmm I don't think so, at least not a reliable test: most of the time the test wouldn't hit a dangerous interleaving. We could try to make the bad concurrent interleaving more likely, or loop a bunch so it's more likely to hit the race because it tries more, but I think fundamentally it would be a flaky "success" test, and mostly a test that does nothing. Were the code to start being broken, it would become a flaky failure.
>>
>>
>> Could you manufacture a deterministic (and dangerous) interleaving, by having the signal handler raise a signal on itself partway through something?
> 
> 
> I don't think that'll do what we want: we want regular code to `raise` at a dangerous spot. We *can* `raise` from the signal handler, but that's not the race that was causing us trouble.

I wondered if that would be a similar enough race that it would require the same sort of careful work, and thus could protect against "simplifying" the code later.  But if that won't do it I suppose we'll just have to trust people to read comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D46858