Page MenuHomeFreeBSD

Fix two issues with Linuxolator signals
ClosedPublic

Authored by kib on Jun 7 2021, 2:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 9:18 AM
Unknown Object (File)
Sun, Nov 3, 12:04 AM
Unknown Object (File)
Thu, Oct 31, 5:37 PM
Unknown Object (File)
Sun, Oct 13, 12:52 PM
Unknown Object (File)
Oct 6 2024, 10:26 PM
Unknown Object (File)
Oct 6 2024, 10:26 PM
Unknown Object (File)
Oct 6 2024, 10:26 PM
Unknown Object (File)
Oct 6 2024, 10:26 PM
Subscribers

Details

Summary

In fact, it also adds two knobs for FreeBSD native signal handling. It makes FreeBSD behavior similar to Linux, but my experiments demonstrated with absolute certainty that turning them by default is out of question. I believe it is still useful to have these knobs available to application porters to confirm that some code expects Linux behavior and needs porting.

  1. Make an option to queue ignored signals instead of dropping them at the moment of generation. This is the most problematic case, lots of code is aware of the differences there, and it manifests itself e.g. in the handling of blocked SIGCHLD.
  2. Make an option for waitpid6(2) and related syscalls to not consume siginfo for queued SIGCHLD from stop events. Apparently Linux/glibc expect that after wait(2), sigtimedwait(SIGCHLD) still returns siginfo for the live process. We have the behavior where wait(2) consumes siginfo, from the inception.

For Linux, I added sysent flags to request the changed behavior unconditionally.

Also, there is a fix for sigtimedwait()/sigwaitinfo() to never restart.

Per commit view: https://kib.kiev.ua/git/gitweb.cgi?p=deviant3.git;a=shortlog;h=refs/heads/sigtimedwait_dfl

Reported by: dchagin

Test Plan

I expect that Dmitry would eventually add his programs to our test suite (module the licensing issues). It might be worth calling the tests with and without knobs turned, to check both behaviors.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib requested review of this revision.Jun 7 2021, 2:59 PM

Add comment for sigwait() about EINTR.

sys/kern/kern_sig.c
1170–1171

Im fine with change, one question - for now kern_sigtimedwait should never return ERESTART, what is the reason to leave it here?

sys/kern/kern_sig.c
1170–1171

The comment right above the block should explain it. Very old libc (before symbol versioning) did not contained the loop over EINTR, this is why we translate EINTR to ERESTART one line above.

Thank you! Ok from me, also, my local repo with this change successfully tinderboxed

This revision is now accepted and ready to land.Jun 8 2021, 2:52 PM
sys/kern/kern_exit.c
109 ↗(On Diff #90525)

bool as well?

sys/kern/kern_sig.c
169

Bikeshedding a bit, but instead of calling it "ignore ignored signals", "discard ignored signals" might be more clear.

1170–1171

But doesn't it mean that we can instead write

if (error == EINTR && td->td_proc->p_osrel < P_OSREL_SIGWAIT)
    return (ERESTART);
td->td_retval[0] = error;
return (0);

?

Or is it still possible for kern_sigtimedwait() to return ERESTART?

1172
3066
kib marked 6 inline comments as done.

Handle Mark' notes.

This revision now requires review to proceed.Jun 14 2021, 5:48 PM
This revision is now accepted and ready to land.Jun 15 2021, 1:58 PM