Avoid EINTR when debugging.

[kib]

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.

[jmg]

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]

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]

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.

[jilles]

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.