Page MenuHomeFreeBSD

Avoid EINTR when debugging.
Needs ReviewPublic

Authored by kib on Sep 13 2015, 1:50 PM.

Details

Reviewers
jmg
jhb
avg
Summary

I found this patch from really old times. I believe what I tried to do is to avoid spurious EINTR from system calls like select(2)/poll(2) and sleeps, which explicitely want to return EINTR for the signal interruption even when the signal disposition is flagged with SA_RESTART.

I have vague memory that the issue was reported by Andrey in context of udtrace, and that the patch has some issues, but I do not remember the outcome. I could revive it if this is considered interesting.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

kib updated this revision to Diff 8717.Sep 13 2015, 1:50 PM
kib retitled this revision from to Avoid EINTR when debugging..
kib updated this object.
kib edited the test plan for this revision. (Show Details)
kib added reviewers: jhb, avg.
kib set the repository for this revision to rS FreeBSD src repository.
jmg edited edge metadata.Oct 20 2015, 11:30 PM

I have no issue w/ the kern_event.c change. I am have not reviewed anything else.

sys/kern/kern_event.c
1531

I have no issue w/ this change.

jilles added a subscriber: jilles.Oct 31 2015, 9:37 PM

Downside of this is that timeouts of the affected calls (kqueue/select/poll/nanosleep) are reset after a debug interrupt. If debug interrupts occur more often than the timeout, the calls will never time out. This is probably less harmful than the current [EINTR] but still a bug.

This can probably be fixed for the most part by storing the original wake up time (absolute sbintime) in struct thread and removing the record when resuming at a different location (in addition to when it is used).

System calls that take an absolute time or usable scratch space to store the remaining time do not have this problem.

sys/kern/sys_process.c
1046

This hunk seems unrelated to the rest of this patch.

kib added a comment.Nov 3 2015, 9:08 AM
In D3654#84710, @jilles wrote:

Downside of this is that timeouts of the affected calls (kqueue/select/poll/nanosleep) are reset after a debug interrupt. If debug interrupts occur more often than the timeout, the calls will never time out. This is probably less harmful than the current [EINTR] but still a bug.
This can probably be fixed for the most part by storing the original wake up time (absolute sbintime) in struct thread and removing the record when resuming at a different location (in addition to when it is used).
System calls that take an absolute time or usable scratch space to store the remaining time do not have this problem.

Yes, the restart problem is there. So, do you agree with having something along this lines (with or without the handling of timeout spent time) ?

sys/kern/sys_process.c
1046

Definitely, thanks for noting.

In D3654#85162, @kib wrote:

Yes, the restart problem is there. So, do you agree with having something along this lines (with or without the handling of timeout spent time) ?

I like the idea of making debugging more transparent to the debugged process but I'm a bit wary of introducing behaviours that may cause other unexpected results. It also has a bit more tentacles into general kernel code than I'd like, but I'm not sure that can be fixed.