On Wed, May 29, 2019 at 9:12 AM Oleg Nesterov oleg@redhat.com wrote:
Al, Linus, Eric, please help.
The previous discussion was very confusing, we simply can not understand each other.
To me everything looks very simple and clear, but perhaps I missed something obvious? Please correct me.
I think that the following code is correct
int interrupted = 0; void sigint_handler(int sig) { interrupted = 1; } int main(void) { sigset_t sigint, empty; sigemptyset(&sigint); sigaddset(&sigint, SIGINT); sigprocmask(SIG_BLOCK, &sigint, NULL); signal(SIGINT, sigint_handler); sigemptyset(&empty); // so pselect() unblocks SIGINT ret = pselect(..., &empty); if (ret >= 0) // sucess or timeout assert(!interrupted); if (interrupted) assert(ret == -EINTR); }
IOW, if pselect(sigmask) temporary unblocks SIGINT according to sigmask, this signal should not be delivered if a ready fd was found or timeout. The signal handle should only run if ret == -EINTR.
I do not think we discussed this part earlier. But, if this is true then this is what is wrong as part of 854a6ed56839a. I missed that before.
(pselect() can be interrupted by any other signal which has a handler. In this case the handler can be called even if ret >= 0. This is correct, I fail to understand why some people think this is wrong, and in any case we simply can't avoid this).
This patch is wrong because I did not know that it was ok to deliver a signal and not set the errno before. I also admitted to this. And proposed another way to revert the patch.: https://lore.kernel.org/lkml/CABeXuvouBzZuNarmNcd9JgZgvonL1N_p21gat=O_x0-1hM...
-Deepa