"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mikolaj Kucharski <mikolaj@kucharski.name>
Subject:
Re: fix tog: poll: Interrupted system call
To:
gameoftrees@openbsd.org
Date:
Wed, 19 May 2021 12:01:55 +0000

Download raw body.

Thread
I cannot really give you a feedback on the diff, as this is far away
from my area of expertise. I can only provide some bike shedding,
nitpicking comments..

On Mon, May 10, 2021 at 01:22:11PM +0200, Stefan Sperling wrote:
> When resizing a terminal window while tog is loading commits the
> following error will trigger occasionally:
> 
> 	tog: poll: Interrupted system call	
> 
> One way of triggering this deliberately is to start tog and resize
> the terminal via keyboard shortcuts immediately. This isn't 100%
> reproducible, but I can usually trigger it after a few attempts.
> 
> The patch below ignores SIGWINCH while polling for a message from a
> privsep helper. I've been unable to trigger the error with this so far.
> 
> There can now be a slight delay before tog's window gets updated after a
> resize event. I think this is acceptable given that the current behaviour
> results in a program exit.
> 
> This code path is also used by got(1) but I don't think that's an issue.
> 
> Is this a correct fix or is there a better solution?
> 
> -----------------------------------------------
> commit 7dd602c8b5180d12decec8796190f9199f68079d (sigwinch)
> from: Stefan Sperling <stsp@stsp.name>
> date: Mon May 10 11:20:45 2021 UTC
>  
>  ignore SIGWINCH while polling in the main process
>  
> diff cffede58c8bec05ab17852de500cfc7225ef8fdc 3e1a4394edfb54bfb6c6119a22436d029c3c68ce
> blob - df3a4a17cfe53a165183f57fb4a0eebe62acab91
> blob + 97229123f0a5c6b797ce37122fe5960b948d80cc
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -22,6 +22,7 @@
>  
>  #include <ctype.h>
>  #include <limits.h>
> +#include <signal.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -59,12 +60,22 @@ static const struct got_error *
>  poll_fd(int fd, int events, int timeout)
>  {
>  	struct pollfd pfd[1];
> +	struct timespec ts;
> +	sigset_t sigset;
>  	int n;
>  
>  	pfd[0].fd = fd;
>  	pfd[0].events = events;
>  
> -	n = poll(pfd, 1, timeout);
> +	ts.tv_sec = timeout;
> +	ts.tv_nsec = 0;
> +
> +	if (sigemptyset(&sigset) == -1)
> +		return got_error_from_errno("sigemptyset");
> +	if (sigaddset(&sigset, SIGWINCH) == -1)
> +		return got_error_from_errno("sigaddset");
> +
> +	n = ppoll(pfd, 1, timeout == INFTIM ? NULL : &ts, &sigset);
>  	if (n == -1)
>  		return got_error_from_errno("poll");

You changed poll(2) to ppoll(2), so maybe above got_error_from_errno()
should also be updated to "ppoll" ?

>  	if (n == 0)
> 

-- 
Regards,
 Mikolaj